diff --git a/src/uu/b2sum/src/b2sum.rs b/src/uu/b2sum/src/b2sum.rs index ddd5fe3e9e6..cc77034f1b3 100644 --- a/src/uu/b2sum/src/b2sum.rs +++ b/src/uu/b2sum/src/b2sum.rs @@ -15,7 +15,10 @@ use uucore::translate; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let calculate_blake2b_length = |s: &str| calculate_blake_length_str(AlgoKind::Blake2b, s); + let calculate_blake2b_length = |n: Option| match n { + None | Some(0) => Ok(None), + Some(n) => calculate_blake_length_str(AlgoKind::Blake2b, &n.to_string()), + }; 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..1531588f3f0 100644 --- a/src/uu/checksum_common/src/cli.rs +++ b/src/uu/checksum_common/src/cli.rs @@ -77,6 +77,8 @@ impl ChecksumCommand for Command { .long(options::LENGTH) .short('l') .help(translate!("ck-common-help-length")) + .value_parser(clap::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 f0d307870e7..0be684591e0 100644 --- a/src/uu/checksum_common/src/lib.rs +++ b/src/uu/checksum_common/src/lib.rs @@ -9,6 +9,7 @@ use std::borrow::Borrow; use std::ffi::OsString; use clap::builder::ValueParser; +use clap::parser::ValueSource; use clap::{Arg, ArgAction, ArgMatches, Command, ValueHint}; use uucore::checksum::compute::{ @@ -63,17 +64,16 @@ pub fn standalone_with_length_main( algo: AlgoKind, cmd: Command, args: impl uucore::Args, - validate_len: fn(&str) -> UResult>, + validate_len: fn(Option) -> 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()? - .flatten(); + #[allow(clippy::unwrap_used, reason = "LENGTH has default_value(\"0\")")] + let raw_length = *matches.get_one::(options::LENGTH).unwrap(); + let input_length = (matches.value_source(options::LENGTH) == Some(ValueSource::CommandLine)) + .then_some(raw_length); + let length = validate_len(input_length)?; //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 19898def4f9..6d6945aefcb 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -7,7 +7,7 @@ use std::io::{Write, stderr}; -use clap::Command; +use clap::{Command, parser::ValueSource}; use uu_checksum_common::{ChecksumCommand, checksum_main, default_checksum_app, options}; use uucore::checksum::compute::OutputFormat; @@ -44,37 +44,38 @@ fn print_cpu_debug_info() { } /// Sanitize the `--length` argument depending on `--algorithm` and `--length`. +/// `input_length` is `None` when no length was provided, or `Some(n)` when +/// the user explicitly passed `--length n`. fn maybe_sanitize_length( algo_cli: Option, - input_length: Option<&str>, + input_length: Option, ) -> 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) + // Note: 0 is explicitly invalid for SHA2/SHA3 (cannot use as default). + (Some(algo @ (AlgoKind::Sha2 | AlgoKind::Sha3)), Some(len)) => { + sanitize_sha2_sha3_length_str(algo, &len.to_string()).map(Some) } // 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), Some(len)) => { + if len == 0 { Ok(None) } else { Ok(Some(len)) } + } // For BLAKE, if a length is provided, validate it. (Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), Some(len)) => { - calculate_blake_length_str(algo, len) + calculate_blake_length_str(algo, &len.to_string()) } - // For any other provided algorithm, check if length is 0. + // For any other provided algorithm, 0 is the alias for default. // Otherwise, this is an error. - (_, Some(len)) if len.parse::() == Ok(0_u32) => Ok(None), + (_, Some(0)) => Ok(None), (_, Some(_)) => Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into()), } } @@ -88,9 +89,10 @@ 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 raw_length = *matches.get_one::(options::LENGTH).unwrap(); + let input_length = (matches.value_source(options::LENGTH) == Some(ValueSource::CommandLine)) + .then_some(raw_length); let length = maybe_sanitize_length(algo_cli, input_length)?; let tag = !matches.get_flag(options::UNTAGGED); diff --git a/tests/by-util/test_b2sum.rs b/tests/by-util/test_b2sum.rs index 2bbc15a8c64..8af8763f0d6 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,25 @@ 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() { + // "18446744073709552000" overflows usize; clap's parser rejects it before our code runs + let len = "18446744073709552000"; + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.write("testf", "foobar\n"); + + scene + .ccmd("b2sum") + .arg("--length") + .arg(len) + .arg(at.subdir.join("testf")) + .fails_with_code(1) + .no_stdout() + .stderr_contains(format!("invalid value '{len}'")); +} + #[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 1ab5bb45285..ecedc251bac 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -300,7 +300,8 @@ fn test_untagged_algorithm_stdin() { #[test] fn test_sha_length_invalid() { for algo in ["sha2", "sha3"] { - for l in ["0", "00", "13", "56", "99999999999999999999999999"] { + // Valid usizes that are invalid lengths for sha2/sha3 + for l in ["0", "13", "56"] { new_ucmd!() .arg("--algorithm") .arg(algo) @@ -332,7 +333,65 @@ fn test_sha_length_invalid() { )); } - // Different error for NaNs + // "00" is parsed as usize 0 by clap's parser; error reports '0' not '00' + for l in ["00"] { + new_ucmd!() + .arg("--algorithm") + .arg(algo) + .arg("--length") + .arg(l) + .arg("/dev/null") + .fails_with_code(1) + .no_stdout() + .stderr_contains("invalid length: '0'") + .stderr_contains(format!( + "digest length for '{}' must be 224, 256, 384, or 512", + algo.to_ascii_uppercase() + )); + + // 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("invalid length: '0'") + .stderr_contains(format!( + "digest length for '{}' must be 224, 256, 384, or 512", + algo.to_ascii_uppercase() + )); + } + + // Overflow values: clap's usize parser rejects these before our code runs + for l in ["99999999999999999999999999"] { + new_ucmd!() + .arg("--algorithm") + .arg(algo) + .arg("--length") + .arg(l) + .arg("/dev/null") + .fails_with_code(1) + .no_stdout() + .stderr_contains(format!("invalid value '{l}'")); + + // 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 value '{l}'")); + } + + // Non-numeric values: clap's usize parser rejects these before our code runs for l in ["512x", "x512", "512x512"] { new_ucmd!() .arg("--algorithm") @@ -342,7 +401,7 @@ fn test_sha_length_invalid() { .arg("/dev/null") .fails_with_code(1) .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")); + .stderr_contains(format!("invalid value '{l}'")); // Also fails with --check new_ucmd!() @@ -354,7 +413,7 @@ fn test_sha_length_invalid() { .arg("--check") .fails_with_code(1) .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")); + .stderr_contains(format!("invalid value '{l}'")); } } } @@ -776,7 +835,8 @@ fn test_blake2b_length() { #[test] fn test_blake2b_length_greater_than_512() { - for l in ["513", "1024", "73786976294838206464"] { + // Valid usizes > 512 + for l in ["513", "1024"] { new_ucmd!() .arg("--algorithm=blake2b") .arg("--length") @@ -787,10 +847,22 @@ fn test_blake2b_length_greater_than_512() { .stderr_contains(format!("invalid length: '{l}'")) .stderr_contains("maximum digest length for 'BLAKE2b' is 512 bits"); } + // Overflow: clap's usize parser rejects this before our code runs + for l in ["73786976294838206464"] { + new_ucmd!() + .arg("--algorithm=blake2b") + .arg("--length") + .arg(l) + .arg("lorem_ipsum.txt") + .fails_with_code(1) + .no_stdout() + .stderr_contains(format!("invalid value '{l}'")); + } } #[test] fn test_blake2b_length_nan() { + // Non-numeric values: clap's usize parser rejects these before our code runs for l in ["foo", "512x", "x512", "0xff"] { new_ucmd!() .arg("--algorithm=blake2b") @@ -799,7 +871,7 @@ fn test_blake2b_length_nan() { .arg("lorem_ipsum.txt") .fails_with_code(1) .no_stdout() - .stderr_contains(format!("invalid length: '{l}'")); + .stderr_contains(format!("invalid value '{l}'")); } } @@ -831,19 +903,32 @@ fn test_blake2b_length_repeated() { #[test] fn test_blake2b_length_invalid() { - for len in [ - "1", "01", // Odd - "", - ] { - new_ucmd!() - .arg("--length") - .arg(len) - .arg("--algorithm=blake2b") - .arg("lorem_ipsum.txt") - .arg("alice_in_wonderland.txt") - .fails_with_code(1) - .stderr_contains(format!("invalid length: '{len}'")); - } + // "1" is a valid usize but not a multiple of 8 + new_ucmd!() + .arg("--length=1") + .arg("--algorithm=blake2b") + .arg("lorem_ipsum.txt") + .arg("alice_in_wonderland.txt") + .fails_with_code(1) + .stderr_contains("invalid length: '1'"); + + // "01" is parsed as usize 1 by clap's parser; error reports '1' not '01' + new_ucmd!() + .arg("--length=01") + .arg("--algorithm=blake2b") + .arg("lorem_ipsum.txt") + .arg("alice_in_wonderland.txt") + .fails_with_code(1) + .stderr_contains("invalid length: '1'"); + + // Empty string: clap's usize parser rejects this + new_ucmd!() + .arg("--length=") + .arg("--algorithm=blake2b") + .arg("lorem_ipsum.txt") + .arg("alice_in_wonderland.txt") + .fails_with_code(1) + .stderr_contains("invalid value ''"); } #[test]