diff --git a/src/uu/b2sum/src/b2sum.rs b/src/uu/b2sum/src/b2sum.rs index 66227664192..ddbfea3bd11 100644 --- a/src/uu/b2sum/src/b2sum.rs +++ b/src/uu/b2sum/src/b2sum.rs @@ -16,7 +16,7 @@ use uucore::translate; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let calculate_blake2b_length = - |s: &str| parse_blake_length(AlgoKind::Blake2b, BlakeLength::String(s)); + |n: usize| parse_blake_length(AlgoKind::Blake2b, BlakeLength::Int(n)); standalone_with_length_main(AlgoKind::Blake2b, uu_app(), args, calculate_blake2b_length) } diff --git a/src/uu/checksum_common/src/cli.rs b/src/uu/checksum_common/src/cli.rs index 50beb3f3b5a..c8e5b480478 100644 --- a/src/uu/checksum_common/src/cli.rs +++ b/src/uu/checksum_common/src/cli.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use clap::{Arg, ArgAction, Command}; +use clap::{Arg, ArgAction, Command, value_parser}; use uucore::{checksum::SUPPORTED_ALGORITHMS, translate}; /// List of all options that can be encountered in checksum utils @@ -77,6 +77,8 @@ impl ChecksumCommand for Command { .long(options::LENGTH) .short('l') .help(translate!("ck-common-help-length")) + .value_parser(value_parser!(usize)) + .default_value("0") .action(ArgAction::Set), ) } diff --git a/src/uu/checksum_common/src/lib.rs b/src/uu/checksum_common/src/lib.rs index 56ded13f19e..4f29db365d6 100644 --- a/src/uu/checksum_common/src/lib.rs +++ b/src/uu/checksum_common/src/lib.rs @@ -63,16 +63,15 @@ pub fn standalone_with_length_main( algo: AlgoKind, cmd: Command, args: impl uucore::Args, - validate_len: fn(&str) -> UResult, + validate_len: fn(usize) -> UResult, ) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(cmd, args)?; let algo = Some(algo); - let length = matches - .get_one::(options::LENGTH) - .map(String::as_str) - .map(validate_len) - .transpose()?; + #[allow(clippy::unwrap_used, reason = "LENGTH has default_value(\"0\")")] + let length = Some(validate_len( + *matches.get_one::(options::LENGTH).unwrap(), + )?); //todo: deduplicate matches.get_flag let text = !matches.get_flag(options::BINARY); diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index df3f46a14a0..c3020f5f8cb 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -44,38 +44,34 @@ fn print_cpu_debug_info() { } /// Sanitize the `--length` argument depending on `--algorithm` and `--length`. -fn maybe_sanitize_length( - algo_cli: Option, - input_length: Option<&str>, -) -> UResult> { +fn maybe_sanitize_length(algo_cli: Option, input_length: usize) -> UResult { match (algo_cli, input_length) { - // No provided length is not a problem so far. - (_, None) => Ok(None), - - // For SHA2 and SHA3, if a length is provided, ensure it is correct. - (Some(algo @ (AlgoKind::Sha2 | AlgoKind::Sha3)), Some(s_len)) => { - sanitize_sha2_sha3_length_str(algo, s_len).map(Some) + // For SHA2 and SHA3, if a non-zero length is provided, ensure it is correct. + // 0 means "not provided" (clap's default_value) — pass it through so that + // from_unsized can emit the appropriate "requires specifying --length" error, + // which also allows --check mode to infer the length from the checksum file. + (Some(algo @ (AlgoKind::Sha2 | AlgoKind::Sha3)), len) => { + if len == 0 { + Ok(0) + } else { + sanitize_sha2_sha3_length_str(algo, &len.to_string()) + } } // SHAKE128 and SHAKE256 algorithms optionally take a bit length. No // validation is performed on this length, any value is valid. If the // given length is not a multiple of 8, the last byte of the output // will have its extra bits set to zero. - (Some(AlgoKind::Shake128 | AlgoKind::Shake256), Some(len)) => match len.parse::() { - Ok(0) => Ok(None), - Ok(l) => Ok(Some(l)), - Err(_) => Err(ChecksumError::InvalidLength(len.into()).into()), - }, + (Some(AlgoKind::Shake128 | AlgoKind::Shake256), len) => Ok(len), // For BLAKE, if a length is provided, validate it. - (Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), Some(len)) => { - parse_blake_length(algo, BlakeLength::String(len)).map(Some) + (Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), len) => { + parse_blake_length(algo, BlakeLength::Int(len)) } - // For any other provided algorithm, check if length is 0. - // Otherwise, this is an error. - (_, Some(len)) if len.parse::() == Ok(0_u32) => Ok(None), - (_, Some(_)) => Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into()), + // For any other algorithm, length must be 0. + (_, 0) => Ok(0), + (_, _) => Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into()), } } @@ -88,11 +84,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(AlgoKind::from_cksum) .transpose()?; - let input_length = matches - .get_one::(options::LENGTH) - .map(String::as_str); + #[allow(clippy::unwrap_used, reason = "LENGTH has default_value(\"0\")")] + let input_length = *matches.get_one::(options::LENGTH).unwrap(); - let length = maybe_sanitize_length(algo_cli, input_length)?; + let length = match maybe_sanitize_length(algo_cli, input_length)? { + 0 => None, + n => Some(n), + }; let tag = !matches.get_flag(options::UNTAGGED); let binary = matches.get_flag(options::BINARY); let text = matches.get_flag(options::TEXT); diff --git a/src/uucore/src/lib/features/checksum/mod.rs b/src/uucore/src/lib/features/checksum/mod.rs index d18e2adaad8..8aa8c462154 100644 --- a/src/uucore/src/lib/features/checksum/mod.rs +++ b/src/uucore/src/lib/features/checksum/mod.rs @@ -276,6 +276,10 @@ pub enum SizedAlgoKind { impl SizedAlgoKind { pub fn from_unsized(kind: AlgoKind, output_length: Option) -> UResult { use AlgoKind as ak; + // 0 is used as a sentinel by callers that use clap's default_value("0") for --length, + // meaning the user did not explicitly provide a length. Normalize Some(0) to None so + // that the match arms below treat it the same as "no length given". + let output_length = output_length.filter(|&n| n != 0); match (kind, output_length) { ( ak::Sysv @@ -509,10 +513,15 @@ pub enum BlakeLength<'s> { pub fn parse_blake_length(algo: AlgoKind, bit_length: BlakeLength<'_>) -> UResult { debug_assert!(matches!(algo, AlgoKind::Blake2b | AlgoKind::Blake3)); + // Previously only handled BlakeLength::String. Extended to BlakeLength::Int so that + // callers passing a parsed usize (via clap's value_parser) also emit the "invalid length" + // error message before the primary error. let print_error = || { - if let BlakeLength::String(s) = bit_length { - show_error!("{}", ChecksumError::InvalidLength(s.to_string())); - } + let s = match bit_length { + BlakeLength::String(s) => s.to_string(), + BlakeLength::Int(i) => i.to_string(), + }; + show_error!("{}", ChecksumError::InvalidLength(s)); }; let n = match bit_length { diff --git a/tests/by-util/test_b2sum.rs b/tests/by-util/test_b2sum.rs index 2bbc15a8c64..4ccbbb1ee5f 100644 --- a/tests/by-util/test_b2sum.rs +++ b/tests/by-util/test_b2sum.rs @@ -177,7 +177,6 @@ fn test_invalid_b2sum_length_option_not_multiple_of_8() { #[rstest] #[case("513")] #[case("1024")] -#[case("18446744073709552000")] fn test_invalid_b2sum_length_option_too_large(#[case] len: &str) { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; @@ -195,6 +194,23 @@ fn test_invalid_b2sum_length_option_too_large(#[case] len: &str) { .stderr_contains("b2sum: maximum digest length for 'BLAKE2b' is 512 bits"); } +#[test] +fn test_invalid_b2sum_length_option_overflow() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.write("testf", "foobar\n"); + + // Overflow is rejected by clap's usize parser + scene + .ccmd("b2sum") + .arg("--length") + .arg("18446744073709552000") + .arg(at.subdir.join("testf")) + .fails_with_code(1) + .no_stdout(); +} + #[test] fn test_check_b2sum_tag_output() { let scene = TestScenario::new(util_name!()); diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 16ed6d8b373..509f219284c 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -312,7 +312,8 @@ fn test_untagged_algorithm_stdin(#[case] algo: &str) { #[test] fn test_sha_length_invalid() { for algo in ["sha2", "sha3"] { - for l in ["0", "00", "13", "56", "99999999999999999999999999"] { + // 0 and 00 are treated as "no length provided" — same error as missing length + for l in ["0", "00"] { new_ucmd!() .arg("--algorithm") .arg(algo) @@ -321,20 +322,19 @@ fn test_sha_length_invalid() { .arg("/dev/null") .fails_with_code(1) .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")) .stderr_contains(format!( - "digest length for '{}' must be 224, 256, 384, or 512", - algo.to_ascii_uppercase() + "--algorithm={algo} requires specifying --length 224, 256, 384, or 512" )); + } - // Also fails with --check + // Non-zero invalid lengths + for l in ["13", "56"] { new_ucmd!() .arg("--algorithm") .arg(algo) .arg("--length") .arg(l) .arg("/dev/null") - .arg("--check") .fails_with_code(1) .no_stdout() .stderr_contains(format!("invalid length: '{l}'")) @@ -342,31 +342,34 @@ fn test_sha_length_invalid() { "digest length for '{}' must be 224, 256, 384, or 512", algo.to_ascii_uppercase() )); - } - // Different error for NaNs - for l in ["512x", "x512", "512x512"] { + // Also fails with --check new_ucmd!() .arg("--algorithm") .arg(algo) .arg("--length") .arg(l) .arg("/dev/null") + .arg("--check") .fails_with_code(1) .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")); + .stderr_contains(format!("invalid length: '{l}'")) + .stderr_contains(format!( + "digest length for '{}' must be 224, 256, 384, or 512", + algo.to_ascii_uppercase() + )); + } - // Also fails with --check + // NaNs and overflow are rejected by clap's usize parser + for l in ["99999999999999999999999999", "512x", "x512", "512x512"] { new_ucmd!() .arg("--algorithm") .arg(algo) .arg("--length") .arg(l) .arg("/dev/null") - .arg("--check") .fails_with_code(1) - .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")); + .no_stdout(); } } } @@ -766,7 +769,7 @@ fn test_blake2b_length() { #[test] fn test_blake2b_length_greater_than_512() { - for l in ["513", "1024", "73786976294838206464"] { + for l in ["513", "1024"] { new_ucmd!() .arg("--algorithm=blake2b") .arg("--length") @@ -777,10 +780,20 @@ fn test_blake2b_length_greater_than_512() { .stderr_contains(format!("invalid length: '{l}'")) .stderr_contains("maximum digest length for 'BLAKE2b' is 512 bits"); } + + // Overflow is rejected by clap's usize parser + new_ucmd!() + .arg("--algorithm=blake2b") + .arg("--length") + .arg("73786976294838206464") + .arg("lorem_ipsum.txt") + .fails_with_code(1) + .no_stdout(); } #[test] fn test_blake2b_length_nan() { + // Non-numeric values are rejected by clap's usize parser for l in ["foo", "512x", "x512", "0xff"] { new_ucmd!() .arg("--algorithm=blake2b") @@ -788,8 +801,7 @@ fn test_blake2b_length_nan() { .arg(l) .arg("lorem_ipsum.txt") .fails_with_code(1) - .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")); + .no_stdout(); } } @@ -821,10 +833,8 @@ fn test_blake2b_length_repeated() { #[test] fn test_blake2b_length_invalid() { - for len in [ - "1", "01", // Odd - "", - ] { + // Non-multiple-of-8 values: "01" parses as 1, so error shows '1' + for len in ["1", "01"] { new_ucmd!() .arg("--length") .arg(len) @@ -832,8 +842,17 @@ fn test_blake2b_length_invalid() { .arg("lorem_ipsum.txt") .arg("alice_in_wonderland.txt") .fails_with_code(1) - .stderr_contains(format!("invalid length: '{len}'")); + .stderr_contains("invalid length: '1'"); } + + // Empty string is rejected by clap's usize parser + new_ucmd!() + .arg("--length") + .arg("") + .arg("--algorithm=blake2b") + .arg("lorem_ipsum.txt") + .fails_with_code(1) + .no_stdout(); } #[apply(test_all_algos)]