Skip to content

Commit ca6d69a

Browse files
cakebakercakebaker
authored
Merge pull request #9063 from RenjiSann/cksum-fix-tests
cksum: Fix GNU `cksum-c.sh` and `cksum-sha3.sh`
2 parents 130bbf5 + e57902f commit ca6d69a

File tree

4 files changed

+275
-93
lines changed

4 files changed

+275
-93
lines changed

‎src/uu/cksum/src/cksum.rs‎

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// spell-checker:ignore (ToDO) fname, algo
77

88
use clap::builder::ValueParser;
9-
use clap::{Arg, ArgAction, Command, value_parser};
9+
use clap::{Arg, ArgAction, Command};
1010
use std::ffi::{OsStr, OsString};
1111
use std::fs::File;
1212
use std::io::{BufReader, Read, Write, stdin, stdout};
@@ -16,8 +16,8 @@ use uucore::checksum::{
1616
ALGORITHM_OPTIONS_BLAKE2B, ALGORITHM_OPTIONS_BSD, ALGORITHM_OPTIONS_CRC,
1717
ALGORITHM_OPTIONS_CRC32B, ALGORITHM_OPTIONS_SHA2, ALGORITHM_OPTIONS_SHA3,
1818
ALGORITHM_OPTIONS_SYSV, ChecksumError, ChecksumOptions, ChecksumVerbose, HashAlgorithm,
19-
LEGACY_ALGORITHMS, SUPPORTED_ALGORITHMS, calculate_blake2b_length, detect_algo, digest_reader,
20-
perform_checksum_validation,
19+
LEGACY_ALGORITHMS, SUPPORTED_ALGORITHMS, calculate_blake2b_length_str, detect_algo,
20+
digest_reader, perform_checksum_validation, sanitize_sha2_sha3_length_str,
2121
};
2222
use uucore::translate;
2323

@@ -186,13 +186,14 @@ fn cksum<'a, I>(mut options: Options, files: I) -> UResult<()>
186186
where
187187
I: Iterator<Item = &'a OsStr>,
188188
{
189-
let files: Vec<_> = files.collect();
189+
let mut files = files.peekable();
190190

191-
if options.output_format.is_raw() && files.len() > 1 {
192-
return Err(Box::new(ChecksumError::RawMultipleFiles));
193-
}
191+
while let Some(filename) = files.next() {
192+
// Check that in raw mode, we are not provided with several files.
193+
if options.output_format.is_raw() && files.peek().is_some() {
194+
return Err(Box::new(ChecksumError::RawMultipleFiles));
195+
}
194196

195-
for filename in files {
196197
let filepath = Path::new(filename);
197198
let stdin_buf;
198199
let file_buf;
@@ -363,42 +364,45 @@ fn figure_out_output_format(
363364
}
364365
}
365366

367+
/// Sanitize the `--length` argument depending on `--algorithm` and `--length`.
368+
fn maybe_sanitize_length(
369+
algo_cli: Option<&str>,
370+
input_length: Option<&str>,
371+
) -> UResult<Option<usize>> {
372+
match (algo_cli, input_length) {
373+
// No provided length is not a problem so far.
374+
(_, None) => Ok(None),
375+
376+
// For SHA2 and SHA3, if a length is provided, ensure it is correct.
377+
(Some(algo @ (ALGORITHM_OPTIONS_SHA2 | ALGORITHM_OPTIONS_SHA3)), Some(s_len)) => {
378+
sanitize_sha2_sha3_length_str(algo, s_len).map(Some)
379+
}
380+
381+
// For BLAKE2b, if a length is provided, validate it.
382+
(Some(ALGORITHM_OPTIONS_BLAKE2B), Some(len)) => calculate_blake2b_length_str(len),
383+
384+
// For any other provided algorithm, check if length is 0.
385+
// Otherwise, this is an error.
386+
(_, Some(len)) if len.parse::<u32>() == Ok(0_u32) => Ok(None),
387+
(_, Some(_)) => Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into()),
388+
}
389+
}
390+
366391
#[uucore::main]
367392
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
368393
let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
369394

370395
let check = matches.get_flag(options::CHECK);
371396

372-
let algo_name: &str = match matches.get_one::<String>(options::ALGORITHM) {
373-
Some(v) => v,
374-
None => {
375-
if check {
376-
// if we are doing a --check, we should not default to crc
377-
""
378-
} else {
379-
ALGORITHM_OPTIONS_CRC
380-
}
381-
}
382-
};
397+
let algo_cli = matches
398+
.get_one::<String>(options::ALGORITHM)
399+
.map(String::as_str);
383400

384-
let input_length = matches.get_one::<usize>(options::LENGTH);
385-
386-
let length = match (input_length, algo_name) {
387-
// Length for sha2 and sha3 should be saved, it will be validated
388-
// afterwards if necessary.
389-
(Some(len), ALGORITHM_OPTIONS_SHA2 | ALGORITHM_OPTIONS_SHA3) => Some(*len),
390-
(None | Some(0), _) => None,
391-
// Length for Blake2b if saved only if it's not zero.
392-
(Some(len), ALGORITHM_OPTIONS_BLAKE2B) => calculate_blake2b_length(*len)?,
393-
// a --length flag set with any other algorithm is an error.
394-
_ => {
395-
return Err(ChecksumError::LengthOnlyForBlake2bSha2Sha3.into());
396-
}
397-
};
401+
let input_length = matches
402+
.get_one::<String>(options::LENGTH)
403+
.map(String::as_str);
398404

399-
if LEGACY_ALGORITHMS.contains(&algo_name) && check {
400-
return Err(ChecksumError::AlgorithmNotSupportedWithCheck.into());
401-
}
405+
let length = maybe_sanitize_length(algo_cli, input_length)?;
402406

403407
let files = matches.get_many::<OsString>(options::FILE).map_or_else(
404408
// No files given, read from stdin.
@@ -408,6 +412,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
408412
);
409413

410414
if check {
415+
// cksum does not support '--check'ing legacy algorithms
416+
if algo_cli.is_some_and(|algo_name| LEGACY_ALGORITHMS.contains(&algo_name)) {
417+
return Err(ChecksumError::AlgorithmNotSupportedWithCheck.into());
418+
}
419+
411420
let text_flag = matches.get_flag(options::TEXT);
412421
let binary_flag = matches.get_flag(options::BINARY);
413422
let strict = matches.get_flag(options::STRICT);
@@ -421,13 +430,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
421430
return Err(ChecksumError::BinaryTextConflict.into());
422431
}
423432

424-
// Determine the appropriate algorithm option to pass
425-
let algo_option = if algo_name.is_empty() {
426-
None
427-
} else {
428-
Some(algo_name)
429-
};
430-
431433
// Execute the checksum validation based on the presence of files or the use of stdin
432434

433435
let verbose = ChecksumVerbose::new(status, quiet, warn);
@@ -438,9 +440,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
438440
verbose,
439441
};
440442

441-
return perform_checksum_validation(files, algo_option, length, opts);
443+
return perform_checksum_validation(files, algo_cli, length, opts);
442444
}
443445

446+
// Not --check
447+
448+
// Set the default algorithm to CRC when not '--check'ing.
449+
let algo_name = algo_cli.unwrap_or(ALGORITHM_OPTIONS_CRC);
450+
444451
let (tag, binary) = handle_tag_text_binary_flags(std::env::args_os())?;
445452

446453
let algo = detect_algo(algo_name, length)?;
@@ -508,7 +515,6 @@ pub fn uu_app() -> Command {
508515
.arg(
509516
Arg::new(options::LENGTH)
510517
.long(options::LENGTH)
511-
.value_parser(value_parser!(usize))
512518
.short('l')
513519
.help(translate!("cksum-help-length"))
514520
.action(ArgAction::Set),

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

Lines changed: 81 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::{
1212
fmt::Display,
1313
fs::File,
1414
io::{self, BufReader, Read, Write, stdin},
15+
num::IntErrorKind,
1516
path::Path,
1617
str,
1718
};
@@ -220,12 +221,12 @@ pub enum ChecksumError {
220221
QuietNotCheck,
221222
#[error("--length required for {}", .0.quote())]
222223
LengthRequired(String),
223-
#[error("unknown algorithm: {0}: clap should have prevented this case")]
224-
UnknownAlgorithm(String),
225-
#[error("length is not a multiple of 8")]
226-
InvalidLength,
224+
#[error("invalid length: {}", .0.quote())]
225+
InvalidLength(String),
227226
#[error("digest length for {} must be 224, 256, 384, or 512", .0.quote())]
228-
InvalidLengthFor(String),
227+
InvalidLengthForSha(String),
228+
#[error("--algorithm={0} requires specifying --length 224, 256, 384, or 512")]
229+
LengthRequiredForSha(String),
229230
#[error("--length is only supported with --algorithm blake2b, sha2, or sha3")]
230231
LengthOnlyForBlake2bSha2Sha3,
231232
#[error("the --binary and --text options are meaningless when verifying checksums")]
@@ -238,6 +239,8 @@ pub enum ChecksumError {
238239
CombineMultipleAlgorithms,
239240
#[error("Needs an algorithm to hash with.\nUse --help for more information.")]
240241
NeedAlgorithmToHash,
242+
#[error("unknown algorithm: {0}: clap should have prevented this case")]
243+
UnknownAlgorithm(String),
241244
#[error("")]
242245
Io(#[from] io::Error),
243246
}
@@ -277,7 +280,7 @@ pub fn create_sha3(bits: usize) -> UResult<HashAlgorithm> {
277280
bits: 512,
278281
}),
279282

280-
_ => Err(ChecksumError::InvalidLengthFor("SHA3".into()).into()),
283+
_ => Err(ChecksumError::InvalidLengthForSha("SHA3".into()).into()),
281284
}
282285
}
283286

@@ -304,7 +307,7 @@ pub fn create_sha2(bits: usize) -> UResult<HashAlgorithm> {
304307
bits: 512,
305308
}),
306309

307-
_ => Err(ChecksumError::InvalidLengthFor("SHA2".into()).into()),
310+
_ => Err(ChecksumError::InvalidLengthForSha("SHA2".into()).into()),
308311
}
309312
}
310313

@@ -837,25 +840,41 @@ fn identify_algo_name_and_length(
837840
last_algo: &mut Option<String>,
838841
) -> Result<(String, Option<usize>), LineCheckError> {
839842
let algo_from_line = line_info.algo_name.clone().unwrap_or_default();
840-
let algorithm = algo_from_line.to_lowercase();
843+
let line_algo = algo_from_line.to_lowercase();
841844
*last_algo = Some(algo_from_line);
842845

843-
// check if we are called with XXXsum (example: md5sum) but we detected a different algo parsing the file
844-
// (for example SHA1 (f) = d...)
846+
// check if we are called with XXXsum (example: md5sum) but we detected a
847+
// different algo parsing the file (for example SHA1 (f) = d...)
848+
//
845849
// Also handle the case cksum -s sm3 but the file contains other formats
846-
if algo_name_input.is_some() && algo_name_input != Some(&algorithm) {
847-
return Err(LineCheckError::ImproperlyFormatted);
850+
if let Some(algo_name_input) = algo_name_input {
851+
match (algo_name_input, line_algo.as_str()) {
852+
(l, r) if l == r => (),
853+
// Edge case for SHA2, which matches SHA(224|256|384|512)
854+
(
855+
ALGORITHM_OPTIONS_SHA2,
856+
ALGORITHM_OPTIONS_SHA224
857+
| ALGORITHM_OPTIONS_SHA256
858+
| ALGORITHM_OPTIONS_SHA384
859+
| ALGORITHM_OPTIONS_SHA512,
860+
) => (),
861+
_ => return Err(LineCheckError::ImproperlyFormatted),
862+
}
848863
}
849864

850-
if !SUPPORTED_ALGORITHMS.contains(&algorithm.as_str()) {
865+
if !SUPPORTED_ALGORITHMS.contains(&line_algo.as_str()) {
851866
// Not supported algo, leave early
852867
return Err(LineCheckError::ImproperlyFormatted);
853868
}
854869

855870
let bytes = if let Some(bitlen) = line_info.algo_bit_len {
856-
match algorithm.as_str() {
871+
match line_algo.as_str() {
857872
ALGORITHM_OPTIONS_BLAKE2B if bitlen % 8 == 0 => Some(bitlen / 8),
858-
ALGORITHM_OPTIONS_SHA3 if [224, 256, 384, 512].contains(&bitlen) => Some(bitlen),
873+
ALGORITHM_OPTIONS_SHA2 | ALGORITHM_OPTIONS_SHA3
874+
if [224, 256, 384, 512].contains(&bitlen) =>
875+
{
876+
Some(bitlen)
877+
}
859878
// Either
860879
// the algo based line is provided with a bit length
861880
// with an algorithm that does not support it (only Blake2B does).
@@ -866,14 +885,14 @@ fn identify_algo_name_and_length(
866885
// the given length is wrong because it's not a multiple of 8.
867886
_ => return Err(LineCheckError::ImproperlyFormatted),
868887
}
869-
} else if algorithm == ALGORITHM_OPTIONS_BLAKE2B {
888+
} else if line_algo == ALGORITHM_OPTIONS_BLAKE2B {
870889
// Default length with BLAKE2b,
871890
Some(64)
872891
} else {
873892
None
874893
};
875894

876-
Ok((algorithm, bytes))
895+
Ok((line_algo, bytes))
877896
}
878897

879898
/// Given a filename and an algorithm, compute the digest and compare it with
@@ -1219,21 +1238,29 @@ pub fn digest_reader<T: Read>(
12191238

12201239
/// Calculates the length of the digest.
12211240
pub fn calculate_blake2b_length(length: usize) -> UResult<Option<usize>> {
1222-
match length {
1223-
0 => Ok(None),
1224-
n if n % 8 != 0 => {
1225-
show_error!("invalid length: \u{2018}{length}\u{2019}");
1241+
calculate_blake2b_length_str(length.to_string().as_str())
1242+
}
1243+
1244+
/// Calculates the length of the digest.
1245+
pub fn calculate_blake2b_length_str(length: &str) -> UResult<Option<usize>> {
1246+
match length.parse() {
1247+
Ok(0) => Ok(None),
1248+
Ok(n) if n % 8 != 0 => {
1249+
show_error!("{}", ChecksumError::InvalidLength(length.into()));
12261250
Err(io::Error::new(io::ErrorKind::InvalidInput, "length is not a multiple of 8").into())
12271251
}
1228-
n if n > 512 => {
1229-
show_error!("invalid length: \u{2018}{length}\u{2019}");
1252+
Ok(n) if n > 512 => {
1253+
show_error!("{}", ChecksumError::InvalidLength(length.into()));
12301254
Err(io::Error::new(
12311255
io::ErrorKind::InvalidInput,
1232-
"maximum digest length for \u{2018}BLAKE2b\u{2019} is 512 bits",
1256+
format!(
1257+
"maximum digest length for {} is 512 bits",
1258+
"BLAKE2b".quote()
1259+
),
12331260
)
12341261
.into())
12351262
}
1236-
n => {
1263+
Ok(n) => {
12371264
// Divide by 8, as our blake2b implementation expects bytes instead of bits.
12381265
if n == 512 {
12391266
// When length is 512, it is blake2b's default.
@@ -1243,17 +1270,43 @@ pub fn calculate_blake2b_length(length: usize) -> UResult<Option<usize>> {
12431270
Ok(Some(n / 8))
12441271
}
12451272
}
1273+
Err(_) => Err(ChecksumError::InvalidLength(length.into()).into()),
12461274
}
12471275
}
12481276

12491277
pub fn validate_sha2_sha3_length(algo_name: &str, length: Option<usize>) -> UResult<usize> {
12501278
match length {
12511279
Some(len @ (224 | 256 | 384 | 512)) => Ok(len),
12521280
Some(len) => {
1253-
show_error!("invalid length: '{len}'");
1254-
Err(ChecksumError::InvalidLengthFor(algo_name.to_ascii_uppercase()).into())
1281+
show_error!("{}", ChecksumError::InvalidLength(len.to_string()));
1282+
Err(ChecksumError::InvalidLengthForSha(algo_name.to_ascii_uppercase()).into())
12551283
}
1256-
None => Err(ChecksumError::LengthRequired(algo_name.to_ascii_uppercase()).into()),
1284+
None => Err(ChecksumError::LengthRequiredForSha(algo_name.into()).into()),
1285+
}
1286+
}
1287+
1288+
pub fn sanitize_sha2_sha3_length_str(algo_name: &str, length: &str) -> UResult<usize> {
1289+
// There is a difference in the errors sent when the length is not a number
1290+
// vs. its an invalid number.
1291+
//
1292+
// When inputting an invalid number, an extra error message it printed to
1293+
// remind of the accepted inputs.
1294+
let len = match length.parse::<usize>() {
1295+
Ok(l) => l,
1296+
// Note: Positive overflow while parsing counts as an invalid number,
1297+
// but a number still.
1298+
Err(e) if *e.kind() == IntErrorKind::PosOverflow => {
1299+
show_error!("{}", ChecksumError::InvalidLength(length.into()));
1300+
return Err(ChecksumError::InvalidLengthForSha(algo_name.to_ascii_uppercase()).into());
1301+
}
1302+
Err(_) => return Err(ChecksumError::InvalidLength(length.into()).into()),
1303+
};
1304+
1305+
if [224, 256, 384, 512].contains(&len) {
1306+
Ok(len)
1307+
} else {
1308+
show_error!("{}", ChecksumError::InvalidLength(length.into()));
1309+
Err(ChecksumError::InvalidLengthForSha(algo_name.to_ascii_uppercase()).into())
12571310
}
12581311
}
12591312

0 commit comments

Comments
 (0)