Skip to content

Commit ec152e4

Browse files
committed
cksum: Use clap's usize parser and default_value("0")
Signed-off-by: Babur Ayanlar <babur.ayanlar@ableton.com>
1 parent 9397a9c commit ec152e4

7 files changed

Lines changed: 102 additions & 59 deletions

File tree

src/uu/b2sum/src/b2sum.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use uucore::translate;
1616
#[uucore::main]
1717
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
1818
let calculate_blake2b_length =
19-
|s: &str| parse_blake_length(AlgoKind::Blake2b, BlakeLength::String(s));
19+
|n: usize| parse_blake_length(AlgoKind::Blake2b, BlakeLength::Int(n));
2020
standalone_with_length_main(AlgoKind::Blake2b, uu_app(), args, calculate_blake2b_length)
2121
}
2222

src/uu/checksum_common/src/cli.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
55

6-
use clap::{Arg, ArgAction, Command};
6+
use clap::{Arg, ArgAction, Command, value_parser};
77
use uucore::{checksum::SUPPORTED_ALGORITHMS, translate};
88

99
/// List of all options that can be encountered in checksum utils
@@ -77,6 +77,8 @@ impl ChecksumCommand for Command {
7777
.long(options::LENGTH)
7878
.short('l')
7979
.help(translate!("ck-common-help-length"))
80+
.value_parser(value_parser!(usize))
81+
.default_value("0")
8082
.action(ArgAction::Set),
8183
)
8284
}

src/uu/checksum_common/src/lib.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,15 @@ pub fn standalone_with_length_main(
6363
algo: AlgoKind,
6464
cmd: Command,
6565
args: impl uucore::Args,
66-
validate_len: fn(&str) -> UResult<usize>,
66+
validate_len: fn(usize) -> UResult<usize>,
6767
) -> UResult<()> {
6868
let matches = uucore::clap_localization::handle_clap_result(cmd, args)?;
6969
let algo = Some(algo);
7070

71-
let length = matches
72-
.get_one::<String>(options::LENGTH)
73-
.map(String::as_str)
74-
.map(validate_len)
75-
.transpose()?;
71+
#[allow(clippy::unwrap_used, reason = "LENGTH has default_value(\"0\")")]
72+
let length = Some(validate_len(
73+
*matches.get_one::<usize>(options::LENGTH).unwrap(),
74+
)?);
7675

7776
//todo: deduplicate matches.get_flag
7877
let text = !matches.get_flag(options::BINARY);

src/uu/cksum/src/cksum.rs

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -44,38 +44,34 @@ fn print_cpu_debug_info() {
4444
}
4545

4646
/// Sanitize the `--length` argument depending on `--algorithm` and `--length`.
47-
fn maybe_sanitize_length(
48-
algo_cli: Option<AlgoKind>,
49-
input_length: Option<&str>,
50-
) -> UResult<Option<usize>> {
47+
fn maybe_sanitize_length(algo_cli: Option<AlgoKind>, input_length: usize) -> UResult<usize> {
5148
match (algo_cli, input_length) {
52-
// No provided length is not a problem so far.
53-
(_, None) => Ok(None),
54-
55-
// For SHA2 and SHA3, if a length is provided, ensure it is correct.
56-
(Some(algo @ (AlgoKind::Sha2 | AlgoKind::Sha3)), Some(s_len)) => {
57-
sanitize_sha2_sha3_length_str(algo, s_len).map(Some)
49+
// For SHA2 and SHA3, if a non-zero length is provided, ensure it is correct.
50+
// 0 means "not provided" (clap's default_value) — pass it through so that
51+
// from_unsized can emit the appropriate "requires specifying --length" error,
52+
// which also allows --check mode to infer the length from the checksum file.
53+
(Some(algo @ (AlgoKind::Sha2 | AlgoKind::Sha3)), len) => {
54+
if len == 0 {
55+
Ok(0)
56+
} else {
57+
sanitize_sha2_sha3_length_str(algo, &len.to_string())
58+
}
5859
}
5960

6061
// SHAKE128 and SHAKE256 algorithms optionally take a bit length. No
6162
// validation is performed on this length, any value is valid. If the
6263
// given length is not a multiple of 8, the last byte of the output
6364
// will have its extra bits set to zero.
64-
(Some(AlgoKind::Shake128 | AlgoKind::Shake256), Some(len)) => match len.parse::<usize>() {
65-
Ok(0) => Ok(None),
66-
Ok(l) => Ok(Some(l)),
67-
Err(_) => Err(ChecksumError::InvalidLength(len.into()).into()),
68-
},
65+
(Some(AlgoKind::Shake128 | AlgoKind::Shake256), len) => Ok(len),
6966

7067
// For BLAKE, if a length is provided, validate it.
71-
(Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), Some(len)) => {
72-
parse_blake_length(algo, BlakeLength::String(len)).map(Some)
68+
(Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), len) => {
69+
parse_blake_length(algo, BlakeLength::Int(len))
7370
}
7471

75-
// For any other provided algorithm, check if length is 0.
76-
// Otherwise, this is an error.
77-
(_, Some(len)) if len.parse::<u32>() == Ok(0_u32) => Ok(None),
78-
(_, Some(_)) => Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into()),
72+
// For any other algorithm, length must be 0.
73+
(_, 0) => Ok(0),
74+
(_, _) => Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into()),
7975
}
8076
}
8177

@@ -88,11 +84,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
8884
.map(AlgoKind::from_cksum)
8985
.transpose()?;
9086

91-
let input_length = matches
92-
.get_one::<String>(options::LENGTH)
93-
.map(String::as_str);
87+
#[allow(clippy::unwrap_used, reason = "LENGTH has default_value(\"0\")")]
88+
let input_length = *matches.get_one::<usize>(options::LENGTH).unwrap();
9489

95-
let length = maybe_sanitize_length(algo_cli, input_length)?;
90+
let length = match maybe_sanitize_length(algo_cli, input_length)? {
91+
0 => None,
92+
n => Some(n),
93+
};
9694
let tag = !matches.get_flag(options::UNTAGGED);
9795
let binary = matches.get_flag(options::BINARY);
9896
let text = matches.get_flag(options::TEXT);

src/uucore/src/lib/features/checksum/mod.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ pub enum SizedAlgoKind {
276276
impl SizedAlgoKind {
277277
pub fn from_unsized(kind: AlgoKind, output_length: Option<usize>) -> UResult<Self> {
278278
use AlgoKind as ak;
279+
// 0 is used as a sentinel by callers that use clap's default_value("0") for --length,
280+
// meaning the user did not explicitly provide a length. Normalize Some(0) to None so
281+
// that the match arms below treat it the same as "no length given".
282+
let output_length = output_length.filter(|&n| n != 0);
279283
match (kind, output_length) {
280284
(
281285
ak::Sysv
@@ -509,10 +513,15 @@ pub enum BlakeLength<'s> {
509513
pub fn parse_blake_length(algo: AlgoKind, bit_length: BlakeLength<'_>) -> UResult<usize> {
510514
debug_assert!(matches!(algo, AlgoKind::Blake2b | AlgoKind::Blake3));
511515

516+
// Previously only handled BlakeLength::String. Extended to BlakeLength::Int so that
517+
// callers passing a parsed usize (via clap's value_parser) also emit the "invalid length"
518+
// error message before the primary error.
512519
let print_error = || {
513-
if let BlakeLength::String(s) = bit_length {
514-
show_error!("{}", ChecksumError::InvalidLength(s.to_string()));
515-
}
520+
let s = match bit_length {
521+
BlakeLength::String(s) => s.to_string(),
522+
BlakeLength::Int(i) => i.to_string(),
523+
};
524+
show_error!("{}", ChecksumError::InvalidLength(s));
516525
};
517526

518527
let n = match bit_length {

tests/by-util/test_b2sum.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ fn test_invalid_b2sum_length_option_not_multiple_of_8() {
177177
#[rstest]
178178
#[case("513")]
179179
#[case("1024")]
180-
#[case("18446744073709552000")]
181180
fn test_invalid_b2sum_length_option_too_large(#[case] len: &str) {
182181
let scene = TestScenario::new(util_name!());
183182
let at = &scene.fixtures;
@@ -195,6 +194,23 @@ fn test_invalid_b2sum_length_option_too_large(#[case] len: &str) {
195194
.stderr_contains("b2sum: maximum digest length for 'BLAKE2b' is 512 bits");
196195
}
197196

197+
#[test]
198+
fn test_invalid_b2sum_length_option_overflow() {
199+
let scene = TestScenario::new(util_name!());
200+
let at = &scene.fixtures;
201+
202+
at.write("testf", "foobar\n");
203+
204+
// Overflow is rejected by clap's usize parser
205+
scene
206+
.ccmd("b2sum")
207+
.arg("--length")
208+
.arg("18446744073709552000")
209+
.arg(at.subdir.join("testf"))
210+
.fails_with_code(1)
211+
.no_stdout();
212+
}
213+
198214
#[test]
199215
fn test_check_b2sum_tag_output() {
200216
let scene = TestScenario::new(util_name!());

tests/by-util/test_cksum.rs

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,8 @@ fn test_untagged_algorithm_stdin(#[case] algo: &str) {
312312
#[test]
313313
fn test_sha_length_invalid() {
314314
for algo in ["sha2", "sha3"] {
315-
for l in ["0", "00", "13", "56", "99999999999999999999999999"] {
315+
// 0 and 00 are treated as "no length provided" — same error as missing length
316+
for l in ["0", "00"] {
316317
new_ucmd!()
317318
.arg("--algorithm")
318319
.arg(algo)
@@ -321,52 +322,54 @@ fn test_sha_length_invalid() {
321322
.arg("/dev/null")
322323
.fails_with_code(1)
323324
.no_stdout()
324-
.stderr_contains(format!("invalid length: '{l}'"))
325325
.stderr_contains(format!(
326-
"digest length for '{}' must be 224, 256, 384, or 512",
327-
algo.to_ascii_uppercase()
326+
"--algorithm={algo} requires specifying --length 224, 256, 384, or 512"
328327
));
328+
}
329329

330-
// Also fails with --check
330+
// Non-zero invalid lengths
331+
for l in ["13", "56"] {
331332
new_ucmd!()
332333
.arg("--algorithm")
333334
.arg(algo)
334335
.arg("--length")
335336
.arg(l)
336337
.arg("/dev/null")
337-
.arg("--check")
338338
.fails_with_code(1)
339339
.no_stdout()
340340
.stderr_contains(format!("invalid length: '{l}'"))
341341
.stderr_contains(format!(
342342
"digest length for '{}' must be 224, 256, 384, or 512",
343343
algo.to_ascii_uppercase()
344344
));
345-
}
346345

347-
// Different error for NaNs
348-
for l in ["512x", "x512", "512x512"] {
346+
// Also fails with --check
349347
new_ucmd!()
350348
.arg("--algorithm")
351349
.arg(algo)
352350
.arg("--length")
353351
.arg(l)
354352
.arg("/dev/null")
353+
.arg("--check")
355354
.fails_with_code(1)
356355
.no_stdout()
357-
.stderr_contains(format!("invalid length: '{l}'"));
356+
.stderr_contains(format!("invalid length: '{l}'"))
357+
.stderr_contains(format!(
358+
"digest length for '{}' must be 224, 256, 384, or 512",
359+
algo.to_ascii_uppercase()
360+
));
361+
}
358362

359-
// Also fails with --check
363+
// NaNs and overflow are rejected by clap's usize parser
364+
for l in ["99999999999999999999999999", "512x", "x512", "512x512"] {
360365
new_ucmd!()
361366
.arg("--algorithm")
362367
.arg(algo)
363368
.arg("--length")
364369
.arg(l)
365370
.arg("/dev/null")
366-
.arg("--check")
367371
.fails_with_code(1)
368-
.no_stdout()
369-
.stderr_contains(format!("invalid length: '{l}'"));
372+
.no_stdout();
370373
}
371374
}
372375
}
@@ -766,7 +769,7 @@ fn test_blake2b_length() {
766769

767770
#[test]
768771
fn test_blake2b_length_greater_than_512() {
769-
for l in ["513", "1024", "73786976294838206464"] {
772+
for l in ["513", "1024"] {
770773
new_ucmd!()
771774
.arg("--algorithm=blake2b")
772775
.arg("--length")
@@ -777,19 +780,28 @@ fn test_blake2b_length_greater_than_512() {
777780
.stderr_contains(format!("invalid length: '{l}'"))
778781
.stderr_contains("maximum digest length for 'BLAKE2b' is 512 bits");
779782
}
783+
784+
// Overflow is rejected by clap's usize parser
785+
new_ucmd!()
786+
.arg("--algorithm=blake2b")
787+
.arg("--length")
788+
.arg("73786976294838206464")
789+
.arg("lorem_ipsum.txt")
790+
.fails_with_code(1)
791+
.no_stdout();
780792
}
781793

782794
#[test]
783795
fn test_blake2b_length_nan() {
796+
// Non-numeric values are rejected by clap's usize parser
784797
for l in ["foo", "512x", "x512", "0xff"] {
785798
new_ucmd!()
786799
.arg("--algorithm=blake2b")
787800
.arg("--length")
788801
.arg(l)
789802
.arg("lorem_ipsum.txt")
790803
.fails_with_code(1)
791-
.no_stdout()
792-
.stderr_contains(format!("invalid length: '{l}'"));
804+
.no_stdout();
793805
}
794806
}
795807

@@ -821,19 +833,26 @@ fn test_blake2b_length_repeated() {
821833

822834
#[test]
823835
fn test_blake2b_length_invalid() {
824-
for len in [
825-
"1", "01", // Odd
826-
"",
827-
] {
836+
// Non-multiple-of-8 values: "01" parses as 1, so error shows '1'
837+
for len in ["1", "01"] {
828838
new_ucmd!()
829839
.arg("--length")
830840
.arg(len)
831841
.arg("--algorithm=blake2b")
832842
.arg("lorem_ipsum.txt")
833843
.arg("alice_in_wonderland.txt")
834844
.fails_with_code(1)
835-
.stderr_contains(format!("invalid length: '{len}'"));
845+
.stderr_contains("invalid length: '1'");
836846
}
847+
848+
// Empty string is rejected by clap's usize parser
849+
new_ucmd!()
850+
.arg("--length")
851+
.arg("")
852+
.arg("--algorithm=blake2b")
853+
.arg("lorem_ipsum.txt")
854+
.fails_with_code(1)
855+
.no_stdout();
837856
}
838857

839858
#[apply(test_all_algos)]

0 commit comments

Comments
 (0)