hashsum: also escape/unescape files with checks (uutils#5868)

* hashsum: make tag conflicts with --text and --check * hashsum: change the case in one of the gnu test * build-gnu.sh: use symlink instead of copy Plus: it won't cp new md5 * hashsum: also escape/unescape files with checks Should fix tests/cksum/md5sum-bsd.sh * improve the variable name Co-authored-by: Daniel Hofstetter <[email protected]> * Improve test Co-authored-by: Daniel Hofstetter <[email protected]> * Improve test Co-authored-by: Daniel Hofstetter <[email protected]> --------- Co-authored-by: Daniel Hofstetter <[email protected]>
cakebaker · Mar 12, 2024 · 1725479 · 1725479
1 parent cc1142c
commit 1725479
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 15 deletions.
diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs
@@ -393,13 +393,15 @@ pub fn uu_app_common() -> Command {
                 .short('c')
                 .long("check")
                 .help("read hashsums from the FILEs and check them")
-                .action(ArgAction::SetTrue),
+                .action(ArgAction::SetTrue)
+                .conflicts_with("tag"),
         )
         .arg(
             Arg::new("tag")
                 .long("tag")
                 .help("create a BSD-style checksum")
-                .action(ArgAction::SetTrue),
+                .action(ArgAction::SetTrue)
+                .conflicts_with("text"),
         )
         .arg(
             Arg::new("text")
@@ -630,7 +632,8 @@ where
             }
             let mut gnu_re = gnu_re_template(&bytes_marker, r"(?P<binary>[ \*])?")?;
             let bsd_re = Regex::new(&format!(
-                r"^{algorithm} \((?P<fileName>.*)\) = (?P<digest>[a-fA-F0-9]{digest_size})",
+                // it can start with \
+                r"^(|\\){algorithm} \((?P<fileName>.*)\) = (?P<digest>[a-fA-F0-9]{digest_size})",
                 algorithm = options.algoname,
                 digest_size = bytes_marker,
             ))
@@ -699,7 +702,8 @@ where
                         }
                     },
                 };
-                let f = match File::open(ck_filename.clone()) {
+                let (ck_filename_unescaped, prefix) = unescape_filename(&ck_filename);
+                let f = match File::open(ck_filename_unescaped) {
                     Err(_) => {
                         failed_open_file += 1;
                         println!(
@@ -732,11 +736,11 @@ where
                 // easier (and more important) on Unix than on Windows.
                 if sum == real_sum {
                     if !options.quiet {
-                        println!("{ck_filename}: OK");
+                        println!("{prefix}{ck_filename}: OK");
                     }
                 } else {
                     if !options.status {
-                        println!("{ck_filename}: FAILED");
+                        println!("{prefix}{ck_filename}: FAILED");
                     }
                     failed_cksum += 1;
                 }
@@ -749,16 +753,19 @@ where
                 options.output_bits,
             )
             .map_err_context(|| "failed to read input".to_string())?;
+            let (escaped_filename, prefix) = escape_filename(filename);
             if options.tag {
-                println!("{} ({}) = {}", options.algoname, filename.display(), sum);
+                println!(
+                    "{}{} ({}) = {}",
+                    prefix, options.algoname, escaped_filename, sum
+                );
             } else if options.nonames {
                 println!("{sum}");
             } else if options.zero {
+                // with zero, we don't escape the filename
                 print!("{} {}{}\0", sum, binary_marker, filename.display());
             } else {
-                let (filename, has_prefix) = escape_filename(filename);
-                let prefix = if has_prefix { "\\" } else { "" };
-                println!("{}{} {}{}", prefix, sum, binary_marker, filename);
+                println!("{}{} {}{}", prefix, sum, binary_marker, escaped_filename);
             }
         }
     }
@@ -783,14 +790,23 @@ where
     Ok(())
 }
 
-fn escape_filename(filename: &Path) -> (String, bool) {
+fn unescape_filename(filename: &str) -> (String, &'static str) {
+    let unescaped = filename
+        .replace("\\\\", "\\")
+        .replace("\\n", "\n")
+        .replace("\\r", "\r");
+    let prefix = if unescaped == filename { "" } else { "\\" };
+    (unescaped, prefix)
+}
+
+fn escape_filename(filename: &Path) -> (String, &'static str) {
     let original = filename.as_os_str().to_string_lossy();
     let escaped = original
         .replace('\\', "\\\\")
         .replace('\n', "\\n")
         .replace('\r', "\\r");
-    let has_changed = escaped != original;
-    (escaped, has_changed)
+    let prefix = if escaped == original { "" } else { "\\" };
+    (escaped, prefix)
 }
 
 fn digest_reader<T: Read>(

diff --git a/tests/by-util/test_hashsum.rs b/tests/by-util/test_hashsum.rs
@@ -356,6 +356,22 @@ fn test_invalid_arg() {
     new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
 }
 
+#[test]
+fn test_conflicting_arg() {
+    new_ucmd!()
+        .arg("--tag")
+        .arg("--check")
+        .arg("--md5")
+        .fails()
+        .code_is(1);
+    new_ucmd!()
+        .arg("--tag")
+        .arg("--text")
+        .arg("--md5")
+        .fails()
+        .code_is(1);
+}
+
 #[test]
 fn test_tag() {
     let scene = TestScenario::new(util_name!());
@@ -386,3 +402,47 @@ fn test_with_escape_filename() {
     assert!(stdout.starts_with('\\'));
     assert!(stdout.trim().ends_with("a\\nb"));
 }
+
+#[test]
+#[cfg(not(windows))]
+fn test_with_escape_filename_zero_text() {
+    let scene = TestScenario::new(util_name!());
+
+    let at = &scene.fixtures;
+    let filename = "a\nb";
+    at.touch(filename);
+    let result = scene
+        .ccmd("md5sum")
+        .arg("--text")
+        .arg("--zero")
+        .arg(filename)
+        .succeeds();
+    let stdout = result.stdout_str();
+    println!("stdout {}", stdout);
+    assert!(!stdout.starts_with('\\'));
+    assert!(stdout.contains("a\nb"));
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_check_with_escape_filename() {
+    let scene = TestScenario::new(util_name!());
+
+    let at = &scene.fixtures;
+
+    let filename = "a\nb";
+    at.touch(filename);
+    let result = scene.ccmd("md5sum").arg("--tag").arg(filename).succeeds();
+    let stdout = result.stdout_str();
+    println!("stdout {}", stdout);
+    assert!(stdout.starts_with("\\MD5"));
+    assert!(stdout.contains("a\\nb"));
+    at.write("check.md5", stdout);
+    let result = scene
+        .ccmd("md5sum")
+        .arg("--strict")
+        .arg("-c")
+        .arg("check.md5")
+        .succeeds();
+    result.stdout_is("\\a\\nb: OK\n");
+}
diff --git a/util/build-gnu.sh b/util/build-gnu.sh
@@ -100,9 +100,9 @@ cp "${UU_BUILD_DIR}/install" "${UU_BUILD_DIR}/ginstall" # The GNU tests rename t
 # Create *sum binaries
 for sum in b2sum b3sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum; do
     sum_path="${UU_BUILD_DIR}/${sum}"
-    test -f "${sum_path}" || cp "${UU_BUILD_DIR}/hashsum" "${sum_path}"
+    test -f "${sum_path}" || (cd ${UU_BUILD_DIR} && ln -s "hashsum" "${sum}")
 done
-test -f "${UU_BUILD_DIR}/[" || cp "${UU_BUILD_DIR}/test" "${UU_BUILD_DIR}/["
+test -f "${UU_BUILD_DIR}/[" || (cd ${UU_BUILD_DIR} && ln -s "test" "[")
 
 ##
 
@@ -329,6 +329,9 @@ ls: invalid --time-style argument 'XX'\nPossible values are: [\"full-iso\", \"lo
 # "hostid BEFORE --help AFTER " same for this
 sed -i -e "s/env \$prog \$BEFORE \$opt > out2/env \$prog \$BEFORE \$opt > out2 #/" -e "s/env \$prog \$BEFORE \$opt AFTER > out3/env \$prog \$BEFORE \$opt AFTER > out3 #/" -e "s/compare exp out2/compare exp out2 #/" -e "s/compare exp out3/compare exp out3 #/" tests/help/help-version-getopt.sh
 
+# The case doesn't really matter here
+sed -i -e "s|WARNING: 1 line is improperly formatted|warning: 1 line is improperly formatted|" tests/cksum/md5sum-bsd.sh
+
 # Add debug info + we have less syscall then GNU's. Adjust our check.
 # Use GNU sed for /c command
 "${SED}" -i -e '/test \$n_stat1 = \$n_stat2 \\/c\