From 39d1713ed04cc44754b6cbdc3f5b6a6f29bd9436 Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Mon, 15 Apr 2024 17:52:06 +0200
Subject: [PATCH] cksum: --tag & --untagged - the order of usage matters.
 manage it

---
 src/uu/cksum/src/cksum.rs   | 182 +++++++++++++++++++++++++++++++-----
 tests/by-util/test_cksum.rs | 153 +++++++++++++++++++++++++++++-
 2 files changed, 305 insertions(+), 30 deletions(-)

diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs
index 4339ff33cc7..3fd7528f5e9 100644
--- a/src/uu/cksum/src/cksum.rs
+++ b/src/uu/cksum/src/cksum.rs
@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) fname, algo
+// spell-checker:ignore (ToDO) fname, algo, asterik
 use clap::{crate_version, value_parser, Arg, ArgAction, Command};
 use hex::decode;
 use hex::encode;
@@ -144,10 +144,10 @@ struct Options {
     algo_name: &'static str,
     digest: Box<dyn Digest + 'static>,
     output_bits: usize,
-    untagged: bool,
+    tag: bool, // will cover the --untagged option
     length: Option<usize>,
     output_format: OutputFormat,
-    binary: bool,
+    asterik: bool, // if we display an asterisk or not (--binary/--text)
 }
 
 /// Calculate checksum
@@ -244,7 +244,7 @@ where
             ),
             (ALGORITHM_OPTIONS_CRC, true) => println!("{sum} {sz}"),
             (ALGORITHM_OPTIONS_CRC, false) => println!("{sum} {sz} {}", filename.display()),
-            (ALGORITHM_OPTIONS_BLAKE2B, _) if !options.untagged => {
+            (ALGORITHM_OPTIONS_BLAKE2B, _) if options.tag => {
                 if let Some(length) = options.length {
                     // Multiply by 8 here, as we want to print the length in bits.
                     println!("BLAKE2b-{} ({}) = {sum}", length * 8, filename.display());
@@ -253,15 +253,15 @@ where
                 }
             }
             _ => {
-                if options.untagged {
-                    let prefix = if options.binary { "*" } else { " " };
-                    println!("{sum} {prefix}{}", filename.display());
-                } else {
+                if options.tag {
                     println!(
                         "{} ({}) = {sum}",
                         options.algo_name.to_ascii_uppercase(),
                         filename.display()
                     );
+                } else {
+                    let prefix = if options.asterik { "*" } else { " " };
+                    println!("{sum} {prefix}{}", filename.display());
                 }
             }
         }
@@ -312,11 +312,75 @@ mod options {
     pub const RAW: &str = "raw";
     pub const BASE64: &str = "base64";
     pub const CHECK: &str = "check";
-    // for legacy compat reasons
     pub const TEXT: &str = "text";
     pub const BINARY: &str = "binary";
 }
 
+/// Determines whether to prompt an asterisk (*) in the output.
+///
+/// This function checks the `tag`, `binary`, and `had_reset` flags and returns a boolean
+/// indicating whether to prompt an asterisk (*) in the output.
+/// It relies on the overrides provided by clap (i.e., `--binary` overrides `--text` and vice versa).
+/// Same for `--tag` and `--untagged`.
+fn prompt_asterisk(tag: bool, binary: bool, had_reset: bool) -> bool {
+    if tag {
+        return false;
+    }
+    if had_reset {
+        return false;
+    }
+    binary
+}
+
+/**
+ * Determine if we had a reset.
+ * This is basically a hack to support the behavior of cksum
+ * when we have the following arguments:
+ * --binary --tag --untagged
+ * Don't do it with clap because if it struggling with the --overrides_with
+ * marking the value as set even if not present
+ */
+fn had_reset(args: &[String]) -> bool {
+    // Indices where "--binary" or "-b", "--tag", and "--untagged" are found
+    let binary_index = args.iter().position(|x| x == "--binary" || x == "-b");
+    let tag_index = args.iter().position(|x| x == "--tag");
+    let untagged_index = args.iter().position(|x| x == "--untagged");
+
+    // Check if all arguments are present and in the correct order
+    match (binary_index, tag_index, untagged_index) {
+        (Some(b), Some(t), Some(u)) => b < t && t < u,
+        _ => false,
+    }
+}
+
+/***
+ * cksum has a bunch of legacy behavior.
+ * We handle this in this function to make sure they are self contained
+ * and "easier" to understand
+ */
+fn handle_tag_text_binary_flags(matches: &clap::ArgMatches, check: bool) -> UResult<(bool, bool)> {
+    let untagged: bool = matches.get_flag(options::UNTAGGED);
+    let tag: bool = matches.get_flag(options::TAG);
+    let tag: bool = tag || !untagged;
+
+    let text_flag: bool = matches.get_flag(options::TEXT);
+    let binary_flag: bool = matches.get_flag(options::BINARY);
+
+    let args: Vec<String> = std::env::args().collect();
+    let had_reset = had_reset(&args);
+
+    let asterik: bool = prompt_asterisk(tag, binary_flag, had_reset);
+
+    if (binary_flag || text_flag) && check {
+        return Err(io::Error::new(
+            io::ErrorKind::InvalidInput,
+            "the --binary and --text options are meaningless when verifying checksums",
+        )
+        .into());
+    }
+    Ok((tag, asterik))
+}
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().try_get_matches_from(args)?;
@@ -369,7 +433,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         None
     };
 
-    if algo_name == "bsd" && check {
+    let (tag, asterik) = handle_tag_text_binary_flags(&matches, check)?;
+
+    if ["bsd", "crc", "sysv"].contains(&algo_name) && check {
         return Err(io::Error::new(
             io::ErrorKind::InvalidInput,
             "--check is not supported with --algorithm={bsd,sysv,crc}",
@@ -377,15 +443,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         .into());
     }
 
-    let untagged: bool = matches.get_flag(options::UNTAGGED);
-    let tag: bool = matches.get_flag(options::TAG);
-
-    let binary = if untagged && tag {
-        false
-    } else {
-        matches.get_flag(options::BINARY)
-    };
-
     let (name, algo, bits) = detect_algo(algo_name, length);
 
     let output_format = if matches.get_flag(options::RAW) {
@@ -401,9 +458,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         digest: algo,
         output_bits: bits,
         length,
-        untagged,
+        tag,
         output_format,
-        binary,
+        asterik,
     };
 
     match matches.get_many::<String>(options::FILE) {
@@ -451,13 +508,15 @@ pub fn uu_app() -> Command {
             Arg::new(options::UNTAGGED)
                 .long(options::UNTAGGED)
                 .help("create a reversed style checksum, without digest type")
-                .action(ArgAction::SetTrue),
+                .action(ArgAction::SetTrue)
+                .overrides_with(options::TAG),
         )
         .arg(
             Arg::new(options::TAG)
                 .long(options::TAG)
                 .help("create a BSD style checksum, undo --untagged (default)")
-                .action(ArgAction::SetTrue),
+                .action(ArgAction::SetTrue)
+                .overrides_with(options::UNTAGGED),
         )
         .arg(
             Arg::new(options::LENGTH)
@@ -503,15 +562,88 @@ pub fn uu_app() -> Command {
             Arg::new(options::TEXT)
                 .long(options::TEXT)
                 .short('t')
-                .hide(true) // for legacy compatibility, no action
+                .hide(true)
+                .overrides_with(options::BINARY)
                 .action(ArgAction::SetTrue),
         )
         .arg(
             Arg::new(options::BINARY)
                 .long(options::BINARY)
                 .short('b')
-                .hide(true) // for legacy compatibility, no action
+                .hide(true)
+                .overrides_with(options::TEXT)
                 .action(ArgAction::SetTrue),
         )
         .after_help(AFTER_HELP)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::had_reset;
+    use crate::prompt_asterisk;
+
+    #[test]
+    fn test_had_reset() {
+        let args = ["--binary", "--tag", "--untagged"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(had_reset(&args));
+
+        let args = ["-b", "--tag", "--untagged"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(had_reset(&args));
+
+        let args = ["-b", "--binary", "--tag", "--untagged"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(had_reset(&args));
+
+        let args = ["--untagged", "--tag", "--binary"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(!had_reset(&args));
+
+        let args = ["--untagged", "--tag", "-b"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(!had_reset(&args));
+
+        let args = ["--binary", "--tag"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(!had_reset(&args));
+
+        let args = ["--tag", "--untagged"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(!had_reset(&args));
+
+        let args = ["--text", "--untagged"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(!had_reset(&args));
+
+        let args = ["--binary", "--untagged"]
+            .iter()
+            .map(|&s| s.to_string())
+            .collect::<Vec<String>>();
+        assert!(!had_reset(&args));
+    }
+
+    #[test]
+    fn test_prompt_asterisk() {
+        assert!(!prompt_asterisk(true, false, false));
+        assert!(!prompt_asterisk(false, false, true));
+        assert!(prompt_asterisk(false, true, false));
+        assert!(!prompt_asterisk(false, false, false));
+    }
+}
diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs
index 31a3f52747f..2352e4981c9 100644
--- a/tests/by-util/test_cksum.rs
+++ b/tests/by-util/test_cksum.rs
@@ -148,7 +148,7 @@ fn test_tag_after_untagged() {
         .arg("-a=md5")
         .arg("lorem_ipsum.txt")
         .succeeds()
-        .stdout_is("cd724690f7dc61775dfac400a71f2caa  lorem_ipsum.txt\n");
+        .stdout_is_fixture("md5_single_file.expected");
 }
 
 #[test]
@@ -235,10 +235,11 @@ fn test_untagged_algorithm_single_file() {
 fn test_tag_short() {
     new_ucmd!()
         .arg("-t")
+        .arg("--untagged")
         .arg("--algorithm=md5")
         .arg("lorem_ipsum.txt")
         .succeeds()
-        .stdout_is("MD5 (lorem_ipsum.txt) = cd724690f7dc61775dfac400a71f2caa\n");
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa  lorem_ipsum.txt\n");
 }
 
 #[test]
@@ -287,6 +288,22 @@ fn test_check_algo() {
         .no_stdout()
         .stderr_contains("cksum: --check is not supported with --algorithm={bsd,sysv,crc}")
         .code_is(1);
+    new_ucmd!()
+        .arg("-a=sysv")
+        .arg("--check")
+        .arg("lorem_ipsum.txt")
+        .fails()
+        .no_stdout()
+        .stderr_contains("cksum: --check is not supported with --algorithm={bsd,sysv,crc}")
+        .code_is(1);
+    new_ucmd!()
+        .arg("-a=crc")
+        .arg("--check")
+        .arg("lorem_ipsum.txt")
+        .fails()
+        .no_stdout()
+        .stderr_contains("cksum: --check is not supported with --algorithm={bsd,sysv,crc}")
+        .code_is(1);
 }
 
 #[test]
@@ -455,6 +472,58 @@ fn test_all_algorithms_fail_on_folder() {
     }
 }
 
+#[cfg(unix)]
+#[test]
+fn test_dev_null() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("f");
+
+    scene
+        .ucmd()
+        .arg("--tag")
+        .arg("--untagged")
+        .arg("--algorithm=md5")
+        .arg(at.subdir.join("/dev/null"))
+        .succeeds()
+        .stdout_contains("d41d8cd98f00b204e9800998ecf8427e ");
+}
+
+#[test]
+fn test_reset_binary() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("f");
+
+    scene
+        .ucmd()
+        .arg("--binary") // should disappear because of the following option
+        .arg("--tag")
+        .arg("--untagged")
+        .arg("--algorithm=md5")
+        .arg(at.subdir.join("f"))
+        .succeeds()
+        .stdout_contains("d41d8cd98f00b204e9800998ecf8427e  ");
+}
+
+#[test]
+fn test_text_tag() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("f");
+
+    scene
+        .ucmd()
+        .arg("--text") // should disappear because of the following option
+        .arg("--tag")
+        .arg(at.subdir.join("f"))
+        .succeeds()
+        .stdout_contains("4294967295 0 ");
+}
+
 #[test]
 fn test_binary_file() {
     let scene = TestScenario::new(util_name!());
@@ -471,16 +540,69 @@ fn test_binary_file() {
         .succeeds()
         .stdout_contains("d41d8cd98f00b204e9800998ecf8427e *");
 
-    // no "*" in this case
     scene
         .ucmd()
-        .arg("--untagged")
         .arg("--tag")
+        .arg("--untagged")
         .arg("--binary")
         .arg("--algorithm=md5")
         .arg(at.subdir.join("f"))
         .succeeds()
-        .stdout_contains("d41d8cd98f00b204e9800998ecf8427e  ");
+        .stdout_contains("d41d8cd98f00b204e9800998ecf8427e *");
+
+    scene
+        .ucmd()
+        .arg("--tag")
+        .arg("--untagged")
+        .arg("--binary")
+        .arg("--algorithm=md5")
+        .arg("raw/blake2b_single_file.expected")
+        .succeeds()
+        .stdout_contains("7e297c07ed8e053600092f91bdd1dad7 *");
+
+    new_ucmd!()
+        .arg("--tag")
+        .arg("--untagged")
+        .arg("--binary")
+        .arg("--algorithm=md5")
+        .arg("lorem_ipsum.txt")
+        .succeeds()
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa *lorem_ipsum.txt\n");
+
+    new_ucmd!()
+        .arg("--untagged")
+        .arg("--binary")
+        .arg("--algorithm=md5")
+        .arg("lorem_ipsum.txt")
+        .succeeds()
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa *lorem_ipsum.txt\n");
+
+    new_ucmd!()
+        .arg("--binary")
+        .arg("--untagged")
+        .arg("--algorithm=md5")
+        .arg("lorem_ipsum.txt")
+        .succeeds()
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa *lorem_ipsum.txt\n");
+
+    new_ucmd!()
+        .arg("-a")
+        .arg("md5")
+        .arg("--binary")
+        .arg("--untagged")
+        .arg("lorem_ipsum.txt")
+        .succeeds()
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa *lorem_ipsum.txt\n");
+
+    new_ucmd!()
+        .arg("-a")
+        .arg("md5")
+        .arg("--binary")
+        .arg("--tag")
+        .arg("--untagged")
+        .arg("lorem_ipsum.txt")
+        .succeeds()
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa  lorem_ipsum.txt\n");
 }
 
 #[test]
@@ -500,3 +622,24 @@ fn test_folder_and_file() {
         .stderr_contains(format!("cksum: {folder_name}: Is a directory"))
         .stdout_is_fixture("crc_single_file.expected");
 }
+
+#[test]
+fn test_conflicting_options() {
+    let scene = TestScenario::new(util_name!());
+
+    let at = &scene.fixtures;
+
+    at.touch("f");
+
+    scene
+        .ucmd()
+        .arg("--binary")
+        .arg("--check")
+        .arg("f")
+        .fails()
+        .no_stdout()
+        .stderr_contains(
+            "cksum: the --binary and --text options are meaningless when verifying checksums",
+        )
+        .code_is(1);
+}