From 74036b1235b1e405a8124727da275687512fba2e Mon Sep 17 00:00:00 2001 From: Babur Ayanlar Date: Tue, 24 Mar 2026 22:26:31 +0100 Subject: [PATCH 1/2] cksum: use clap's usize parser and default_value("0") for --length Replace manual string parsing of --length with clap's built-in usize value_parser. Use value_source to distinguish user-provided values from the default, so Option semantics are preserved without needing to handle Option<&str> chains. --- src/uu/b2sum/src/b2sum.rs | 5 ++++- src/uu/checksum_common/src/cli.rs | 2 ++ src/uu/checksum_common/src/lib.rs | 14 +++++++------- src/uu/cksum/src/cksum.rs | 32 ++++++++++++++++--------------- 4 files changed, 30 insertions(+), 23 deletions(-) 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); From b2fa7dffe2352fe9053a014bf25d35567f543a3c Mon Sep 17 00:00:00 2001 From: Babur Ayanlar Date: Tue, 24 Mar 2026 22:26:43 +0100 Subject: [PATCH 2/2] tests(cksum,b2sum): update length error checks for clap usize parser Clap's usize parser now handles NaN and overflow inputs, producing 'invalid value' errors instead of 'invalid length'. Leading zeros like '00' and '01' parse to their canonical usize values. --- tests/by-util/test_b2sum.rs | 20 +++++- tests/by-util/test_cksum.rs | 123 ++++++++++++++++++++++++++++++------ 2 files changed, 123 insertions(+), 20 deletions(-) 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]