From 1725479c06e111e435d3f3d384701b43d88d1eca Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 12 Mar 2024 14:56:27 +0100 Subject: [PATCH] hashsum: also escape/unescape files with checks (#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 * Improve test Co-authored-by: Daniel Hofstetter * Improve test Co-authored-by: Daniel Hofstetter --------- Co-authored-by: Daniel Hofstetter --- src/uu/hashsum/src/hashsum.rs | 42 ++++++++++++++++-------- tests/by-util/test_hashsum.rs | 60 +++++++++++++++++++++++++++++++++++ util/build-gnu.sh | 7 ++-- 3 files changed, 94 insertions(+), 15 deletions(-) diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs index 5e439853a5..8f57522d62 100644 --- 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[ \*])?")?; let bsd_re = Regex::new(&format!( - r"^{algorithm} \((?P.*)\) = (?P[a-fA-F0-9]{digest_size})", + // it can start with \ + r"^(|\\){algorithm} \((?P.*)\) = (?P[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( diff --git a/tests/by-util/test_hashsum.rs b/tests/by-util/test_hashsum.rs index 7edd387c32..22a028a320 100644 --- 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 index 9fdb3079d9..324f3ed686 100755 --- 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\