cksum: Fix file quoting on stderr (#8272)

* cksum: handle escaping in stderr for file errors

* uutests: Minor fix and stdX_contains_bytes feature

* test(cksum): add test for stderr escaping

Author: RenjiSann

src/uucore/Cargo.toml

@@ -93,7 +93,7 @@ default = []
 # * non-default features
 backup-control = []
 colors = []
-checksum = ["data-encoding", "sum"]
+checksum = ["data-encoding", "quoting-style", "sum"]
 encoding = ["data-encoding", "data-encoding-macro", "z85"]
 entries = ["libc"]
 extendedbigdecimal = ["bigdecimal", "num-traits"]

src/uucore/src/lib/features/checksum.rs

@@ -18,7 +18,9 @@ use std::{
 
 use crate::{
     error::{FromIo, UError, UResult, USimpleError},
-    os_str_as_bytes, os_str_from_bytes, read_os_string_lines, show, show_error, show_warning_caps,
+    os_str_as_bytes, os_str_from_bytes,
+    quoting_style::{QuotingStyle, locale_aware_escape_name},
+    read_os_string_lines, show, show_error, show_warning_caps,
     sum::{
         Blake2b, Blake3, Bsd, CRC32B, Crc, Digest, DigestWriter, Md5, Sha1, Sha3_224, Sha3_256,
         Sha3_384, Sha3_512, Sha224, Sha256, Sha384, Sha512, Shake128, Shake256, Sm3, SysV,
@@ -734,7 +736,7 @@ fn get_file_to_check(
     opts: ChecksumOptions,
 ) -> Result<Box<dyn Read>, LineCheckError> {
     let filename_bytes = os_str_as_bytes(filename).expect("UTF-8 error");
-    let filename_lossy = String::from_utf8_lossy(filename_bytes);
+
     if filename == "-" {
         Ok(Box::new(stdin())) // Use stdin if "-" is specified in the checksum file
     } else {
@@ -747,15 +749,23 @@ fn get_file_to_check(
                 opts.verbose,
             );
         };
+        let print_error = |err: io::Error| {
+            show!(err.map_err_context(|| {
+                locale_aware_escape_name(filename, QuotingStyle::SHELL_ESCAPE)
+                    // This is non destructive thanks to the escaping
+                    .to_string_lossy()
+                    .to_string()
+            }));
+        };
         match File::open(filename) {
             Ok(f) => {
                 if f.metadata()
                     .map_err(|_| LineCheckError::CantOpenFile)?
                     .is_dir()
                 {
-                    show!(USimpleError::new(
-                        1,
-                        format!("{filename_lossy}: Is a directory")
+                    print_error(io::Error::new(
+                        io::ErrorKind::IsADirectory,
+                        "Is a directory",
                     ));
                     // also regarded as a failed open
                     failed_open();
@@ -767,7 +777,7 @@ fn get_file_to_check(
             Err(err) => {
                 if !opts.ignore_missing {
                     // yes, we have both stderr and stdout here
-                    show!(err.map_err_context(|| filename_lossy.to_string()));
+                    print_error(err);
                     failed_open();
                 }
                 // we could not open the file but we want to continue

tests/by-util/test_cksum.rs

@@ -1453,7 +1453,7 @@ fn test_check_trailing_space_fails() {
 /// in checksum files.
 /// These tests are excluded from Windows because it does not provide any safe
 /// conversion between `OsString` and byte sequences for non-utf-8 strings.
-mod check_utf8 {
+mod check_encoding {
 
     // This test should pass on linux and macos.
     #[cfg(not(windows))]
@@ -1467,15 +1467,12 @@ mod check_utf8 {
         BLAKE2b (empty) = eGoC90IBWQPGxv2FJVLScpEvR0DhWEdhiobiF/cfVBnSXhAxr+5YUxOJZESTTrBLkDpoWxRIt1XVb3Aa/pvizg==\n"
     ;
 
-        let scene = TestScenario::new(util_name!());
-        let at = &scene.fixtures;
+        let (at, mut cmd) = at_and_ucmd!();
 
         at.touch("empty");
         at.write_bytes("check", hashes);
 
-        scene
-            .ucmd()
-            .arg("--check")
+        cmd.arg("--check")
             .arg(at.subdir.join("check"))
             .succeeds()
             .stdout_is("empty: OK\nempty: OK\nempty: OK\n")
@@ -1528,6 +1525,29 @@ mod check_utf8 {
             .stdout_is_bytes(b"flakey\xffname: FAILED open or read\n")
             .stderr_contains("1 listed file could not be read");
     }
+
+    #[cfg(target_os = "linux")]
+    #[test]
+    fn test_quoting_in_stderr() {
+        use super::*;
+        use std::{ffi::OsStr, os::unix::ffi::OsStrExt};
+
+        let (at, mut cmd) = at_and_ucmd!();
+
+        at.mkdir(<OsStr as OsStrExt>::from_bytes(b"FFF\xffDIR"));
+        at.write_bytes(
+            "check",
+            b"SHA256 (FFF\xffFFF) = 29953405eaa3dcc41c37d1621d55b6a47eee93e05613e439e73295029740b10c\nSHA256 (FFF\xffDIR) = 29953405eaa3dcc41c37d1621d55b6a47eee93e05613e439e73295029740b10c\n",
+        );
+
+        cmd.arg("-c")
+            .arg("check")
+            .fails_with_code(1)
+            .stdout_contains_bytes(b"FFF\xffFFF: FAILED open or read")
+            .stdout_contains_bytes(b"FFF\xffDIR: FAILED open or read")
+            .stderr_contains("'FFF'$'\\377''FFF': No such file or directory")
+            .stderr_contains("'FFF'$'\\377''DIR': Is a directory");
+    }
 }
 
 #[test]

tests/uutests/src/lib/macros.rs

@@ -71,7 +71,7 @@ macro_rules! new_ucmd {
 #[macro_export]
 macro_rules! at_and_ucmd {
     () => {{
-        let ts = TestScenario::new(util_name!());
+        let ts = ::uutests::util::TestScenario::new(::uutests::util_name!());
         (ts.fixtures.clone(), ts.ucmd())
     }};
 }

tests/uutests/src/lib/util.rs

@@ -12,6 +12,7 @@
     clippy::missing_errors_doc
 )]
 
+use core::str;
 #[cfg(unix)]
 use libc::mode_t;
 #[cfg(unix)]
@@ -758,6 +759,29 @@ impl CmdResult {
         self
     }
 
+    /// Verify if stdout contains a byte sequence
+    ///
+    /// # Examples
+    ///
+    /// ```rust,ignore
+    /// new_ucmd!()
+    /// .arg("--help")
+    /// .succeeds()
+    /// .stdout_contains_bytes(b"hello \xff");
+    /// ```
+    #[track_caller]
+    pub fn stdout_contains_bytes<T: AsRef<[u8]>>(&self, cmp: T) -> &Self {
+        assert!(
+            self.stdout()
+                .windows(cmp.as_ref().len())
+                .any(|sub| sub == cmp.as_ref()),
+            "'{:?}'\ndoes not contain\n'{:?}'",
+            self.stdout(),
+            cmp.as_ref()
+        );
+        self
+    }
+
     /// Verify if stderr contains a specific string
     ///
     /// # Examples
@@ -780,6 +804,29 @@ impl CmdResult {
         self
     }
 
+    /// Verify if stderr contains a byte sequence
+    ///
+    /// # Examples
+    ///
+    /// ```rust,ignore
+    /// new_ucmd!()
+    /// .arg("--help")
+    /// .succeeds()
+    /// .stdout_contains_bytes(b"hello \xff");
+    /// ```
+    #[track_caller]
+    pub fn stderr_contains_bytes<T: AsRef<[u8]>>(&self, cmp: T) -> &Self {
+        assert!(
+            self.stderr()
+                .windows(cmp.as_ref().len())
+                .any(|sub| sub == cmp.as_ref()),
+            "'{:?}'\ndoes not contain\n'{:?}'",
+            self.stderr(),
+            cmp.as_ref()
+        );
+        self
+    }
+
     /// Verify if stdout does not contain a specific string
     ///
     /// # Examples