cksum: rework blake length validation , add tests

src/uu/b2sum/src/b2sum.rs

@@ -9,13 +9,14 @@ use clap::Command;
 
 use uu_checksum_common::{standalone_checksum_app_with_length, standalone_with_length_main};
 
-use uucore::checksum::{AlgoKind, calculate_blake_length_str};
+use uucore::checksum::{AlgoKind, BlakeLength, validate_calculate_blake_length};
 use uucore::error::UResult;
 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 =
+        |s: &str| validate_calculate_blake_length(AlgoKind::Blake2b, BlakeLength::String(s));
     standalone_with_length_main(AlgoKind::Blake2b, uu_app(), args, calculate_blake2b_length)
 }
 

src/uu/checksum_common/src/lib.rs

@@ -63,7 +63,7 @@ pub fn standalone_with_length_main(
     algo: AlgoKind,
     cmd: Command,
     args: impl uucore::Args,
-    validate_len: fn(&str) -> UResult<Option<usize>>,
+    validate_len: fn(&str) -> UResult<usize>,
 ) -> UResult<()> {
     let matches = uucore::clap_localization::handle_clap_result(cmd, args)?;
     let algo = Some(algo);
@@ -72,8 +72,7 @@ pub fn standalone_with_length_main(
         .get_one::<String>(options::LENGTH)
         .map(String::as_str)
         .map(validate_len)
-        .transpose()?
-        .flatten();
+        .transpose()?;
 
     //todo: deduplicate matches.get_flag
     let text = !matches.get_flag(options::BINARY);

src/uu/cksum/src/cksum.rs

@@ -12,7 +12,8 @@ use uu_checksum_common::{ChecksumCommand, checksum_main, default_checksum_app, o
 
 use uucore::checksum::compute::OutputFormat;
 use uucore::checksum::{
-    AlgoKind, ChecksumError, calculate_blake_length_str, sanitize_sha2_sha3_length_str,
+    AlgoKind, BlakeLength, ChecksumError, sanitize_sha2_sha3_length_str,
+    validate_calculate_blake_length,
 };
 use uucore::error::UResult;
 use uucore::hardware::{HasHardwareFeatures as _, SimdPolicy};
@@ -69,7 +70,7 @@ fn maybe_sanitize_length(
 
         // For BLAKE, if a length is provided, validate it.
         (Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), Some(len)) => {
-            calculate_blake_length_str(algo, len)
+            validate_calculate_blake_length(algo, BlakeLength::String(len)).map(Some)
         }
 
         // For any other provided algorithm, check if length is 0.

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

@@ -250,8 +250,8 @@ pub enum SizedAlgoKind {
     Sha2(ShaLength),
     Sha3(ShaLength),
     // Note: we store Blake*'s length as BYTES.
-    Blake2b(Option<usize>),
-    Blake3(Option<usize>),
+    Blake2b(usize),
+    Blake3(usize),
     // Shake* length are stored in bits.
     Shake128(Option<usize>),
     Shake256(Option<usize>),
@@ -284,22 +284,15 @@ impl SizedAlgoKind {
             (ak::Sm3, _) => Ok(Self::Sm3),
             (ak::Sha1, _) => Ok(Self::Sha1),
 
-            (ak::Blake3, l) => Ok(Self::Blake3(l)),
+            (ak::Blake2b, l) => Ok(Self::Blake2b(l.unwrap_or(Blake2b::DEFAULT_BYTE_SIZE))),
+            (ak::Blake3, l) => Ok(Self::Blake3(l.unwrap_or(Blake3::DEFAULT_BYTE_SIZE))),
             (ak::Shake128, l) => Ok(Self::Shake128(l)),
             (ak::Shake256, l) => Ok(Self::Shake256(l)),
             (ak::Sha2, Some(l)) => Ok(Self::Sha2(ShaLength::try_from(l)?)),
             (ak::Sha3, Some(l)) => Ok(Self::Sha3(ShaLength::try_from(l)?)),
             (algo @ (ak::Sha2 | ak::Sha3), None) => {
                 Err(ChecksumError::LengthRequiredForSha(algo.to_lowercase().into()).into())
             }
-            // [`calculate_blake2b_length`] expects a length in bits but we
-            // have a length in bytes.
-            (algo @ ak::Blake2b, Some(l)) => Ok(Self::Blake2b(calculate_blake_length_str(
-                algo,
-                &(8 * l).to_string(),
-            )?)),
-            (ak::Blake2b, None) => Ok(Self::Blake2b(None)),
-
             (ak::Sha224, None) => Ok(Self::Sha2(ShaLength::Len224)),
             (ak::Sha256, None) => Ok(Self::Sha2(ShaLength::Len256)),
             (ak::Sha384, None) => Ok(Self::Sha2(ShaLength::Len384)),
@@ -314,14 +307,9 @@ impl SizedAlgoKind {
             Self::Sha1 => "SHA1".into(),
             Self::Sha2(len) => format!("SHA{}", len.as_usize()),
             Self::Sha3(len) => format!("SHA3-{}", len.as_usize()),
-            Self::Blake2b(Some(byte_len)) => format!("BLAKE2b-{}", byte_len * 8),
-            Self::Blake2b(None) => "BLAKE2b".into(),
-            Self::Blake3(byte_len) => {
-                format!(
-                    "BLAKE3-{}",
-                    byte_len.unwrap_or(Blake3::DEFAULT_BYTE_SIZE) * 8
-                )
-            }
+            Self::Blake2b(Blake2b::DEFAULT_BYTE_SIZE) => "BLAKE2b".into(),
+            Self::Blake2b(byte_len) => format!("BLAKE2b-{}", byte_len * 8),
+            Self::Blake3(byte_len) => format!("BLAKE3-{}", byte_len * 8),
             Self::Shake128(opt_bit_len) => format!(
                 "SHAKE128-{}",
                 opt_bit_len.unwrap_or(Shake128::DEFAULT_BIT_SIZE)
@@ -354,12 +342,8 @@ impl SizedAlgoKind {
             Self::Sha3(Len256) => Box::new(Sha3_256::default()),
             Self::Sha3(Len384) => Box::new(Sha3_384::default()),
             Self::Sha3(Len512) => Box::new(Sha3_512::default()),
-            Self::Blake2b(len_opt) => {
-                Box::new(len_opt.map(Blake2b::with_output_bytes).unwrap_or_default())
-            }
-            Self::Blake3(len_opt) => {
-                Box::new(len_opt.map(Blake3::with_output_bytes).unwrap_or_default())
-            }
+            Self::Blake2b(len) => Box::new(Blake2b::with_output_bytes(*len)),
+            Self::Blake3(len) => Box::new(Blake3::with_output_bytes(*len)),
             Self::Shake128(len_opt) => {
                 Box::new(len_opt.map(Shake128::with_output_bits).unwrap_or_default())
             }
@@ -378,10 +362,10 @@ impl SizedAlgoKind {
             Self::Md5 => 128,
             Self::Sm3 => 512,
             Self::Sha1 => 160,
-            Self::Blake3(len) => len.unwrap_or(Blake3::DEFAULT_BYTE_SIZE) * 8,
             Self::Sha2(len) => len.as_usize(),
             Self::Sha3(len) => len.as_usize(),
-            Self::Blake2b(len) => len.unwrap_or(Blake2b::DEFAULT_BYTE_SIZE * 8),
+            Self::Blake2b(len) => len * 8,
+            Self::Blake3(len) => len * 8,
             Self::Shake128(len) => len.unwrap_or(Shake128::DEFAULT_BIT_SIZE),
             Self::Shake256(len) => len.unwrap_or(Shake256::DEFAULT_BIT_SIZE),
         }
@@ -495,36 +479,65 @@ pub fn digest_reader<T: Read>(
     Ok((digest.result(), output_size))
 }
 
-/// Calculates the BYTE length of the digest.
-pub fn calculate_blake_length_str(algo: AlgoKind, bit_length: &str) -> UResult<Option<usize>> {
+pub enum BlakeLength<'s> {
+    Int(usize),
+    String(&'s str),
+}
+
+/// Expects a size in BITS, either as a string or int, and returns it as a BYTE
+/// length.
+///
+/// Note: when the input is a string, validation may print error messages.
+/// Note: when the algo is Blake2b, values that are above 512
+/// (Blake2b::DEFAULT_BIT_SIZE) are errors.
+pub fn validate_calculate_blake_length(
+    algo: AlgoKind,
+    bit_length: BlakeLength<'_>,
+) -> UResult<usize> {
     debug_assert!(matches!(algo, AlgoKind::Blake2b | AlgoKind::Blake3));
 
+    // TODO MSRV>=1.89 : Replace format! with format_args! to make string
+    // evaluation lazy.
+    #[allow(clippy::useless_format)]
+    let (disp, parsed, may_print_error) = match &bit_length {
+        BlakeLength::Int(i) => (format!("{}", *i), Ok(*i), false),
+        BlakeLength::String(s) => (format!("{}", *s), s.parse::<usize>(), true),
+    };
+
+    let print_error = || {
+        if may_print_error {
+            show_error!("{}", ChecksumError::InvalidLength(disp.clone()));
+        }
+    };
+
     // Blake2b's length is parsed in an u64.
-    match bit_length.parse::<usize>() {
-        Ok(0) => Ok(None),
+    match parsed {
+        Ok(0) => Ok(if algo == AlgoKind::Blake2b {
+            Blake2b::DEFAULT_BYTE_SIZE
+        } else {
+            Blake3::DEFAULT_BYTE_SIZE
+        }),
 
         // Error cases
-        Ok(n) if n > 512 && algo == AlgoKind::Blake2b => {
-            show_error!("{}", ChecksumError::InvalidLength(bit_length.into()));
+        Ok(n) if n > Blake2b::DEFAULT_BIT_SIZE && algo == AlgoKind::Blake2b => {
+            print_error();
             Err(ChecksumError::LengthTooBigForBlake(algo.to_uppercase().into()).into())
         }
         Err(e) if *e.kind() == IntErrorKind::PosOverflow => {
-            show_error!("{}", ChecksumError::InvalidLength(bit_length.into()));
+            print_error();
             Err(ChecksumError::LengthTooBigForBlake(algo.to_uppercase().into()).into())
         }
-        Err(_) => Err(ChecksumError::InvalidLength(bit_length.into()).into()),
+        Err(_) => Err(ChecksumError::InvalidLength(disp).into()),
 
         Ok(n) if n % 8 != 0 => {
-            show_error!("{}", ChecksumError::InvalidLength(bit_length.into()));
+            print_error();
             Err(ChecksumError::LengthNotMultipleOf8.into())
         }
 
         // Valid cases
 
-        // When length is 512, it is blake2b's default. So, don't show it
-        Ok(512) => Ok(None),
         // Divide by 8, as our blake2b implementation expects bytes instead of bits.
-        Ok(n) => Ok(Some(n / 8)),
+        Ok(n) => Ok(n / 8),
     }
 }
 
@@ -644,18 +657,22 @@ mod tests {
     #[test]
     fn test_calculate_blake2b_length() {
         assert_eq!(
-            calculate_blake_length_str(AlgoKind::Blake2b, "0").unwrap(),
-            None
+            validate_calculate_blake_length(AlgoKind::Blake2b, BlakeLength::String("0")).unwrap(),
+            Blake2b::DEFAULT_BYTE_SIZE
+        );
+        assert!(
+            validate_calculate_blake_length(AlgoKind::Blake2b, BlakeLength::String("10")).is_err()
+        );
+        assert!(
+            validate_calculate_blake_length(AlgoKind::Blake2b, BlakeLength::String("520")).is_err()
         );
-        assert!(calculate_blake_length_str(AlgoKind::Blake2b, "10").is_err());
-        assert!(calculate_blake_length_str(AlgoKind::Blake2b, "520").is_err());
         assert_eq!(
-            calculate_blake_length_str(AlgoKind::Blake2b, "512").unwrap(),
-            None
+            validate_calculate_blake_length(AlgoKind::Blake2b, BlakeLength::String("512")).unwrap(),
+            Blake2b::DEFAULT_BYTE_SIZE
         );
         assert_eq!(
-            calculate_blake_length_str(AlgoKind::Blake2b, "256").unwrap(),
-            Some(32)
+            validate_calculate_blake_length(AlgoKind::Blake2b, BlakeLength::String("256")).unwrap(),
+            32
         );
     }
 }

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

@@ -15,11 +15,12 @@ use std::io::{self, BufReader, Read, Write, stderr, stdin};
 use os_display::Quotable;
 
 use crate::checksum::{
-    AlgoKind, ChecksumError, ReadingMode, SizedAlgoKind, digest_reader, unescape_filename,
+    AlgoKind, BlakeLength, ChecksumError, ReadingMode, SizedAlgoKind, digest_reader,
+    unescape_filename, validate_calculate_blake_length,
 };
 use crate::error::{FromIo, UError, UIoError, UResult, USimpleError};
 use crate::quoting_style::{QuotingStyle, locale_aware_escape_name};
-use crate::sum::{self, DigestOutput};
+use crate::sum::{self, Blake2b, Blake3, DigestOutput};
 use crate::{
     os_str_as_bytes, os_str_from_bytes, read_os_string_lines, show, show_warning_caps, translate,
 };
@@ -637,7 +638,12 @@ fn identify_algo_name_and_length(
 
     let bytes = if let Some(bitlen) = line_info.algo_bit_len {
         match line_algo {
-            ak::Blake2b | ak::Blake3 if bitlen % 8 == 0 => Some(bitlen / 8),
+            algo @ (ak::Blake2b | ak::Blake3) => {
+                match validate_calculate_blake_length(algo, BlakeLength::Int(bitlen)) {
+                    Ok(len) => Some(len),
+                    Err(_) => return Err(LineCheckError::ImproperlyFormatted),
+                }
+            }
             ak::Sha2 | ak::Sha3 if [224, 256, 384, 512].contains(&bitlen) => Some(bitlen),
             ak::Shake128 | ak::Shake256 => Some(bitlen),
             // Either
@@ -653,10 +659,10 @@ fn identify_algo_name_and_length(
         }
     } else if line_algo == ak::Blake2b {
         // Default length with BLAKE2b,
-        Some(64)
+        Some(Blake2b::DEFAULT_BYTE_SIZE)
     } else if line_algo == ak::Blake3 {
         // Default length with BLAKE3,
-        Some(32)
+        Some(Blake3::DEFAULT_BYTE_SIZE)
     } else {
         None
     };

src/uucore/src/lib/features/sum.rs

@@ -83,9 +83,15 @@ pub struct Blake2b {
 
 impl Blake2b {
     pub const DEFAULT_BYTE_SIZE: usize = 64;
+    pub const DEFAULT_BIT_SIZE: usize = Self::DEFAULT_BYTE_SIZE * 8;
 
     /// Return a new Blake2b instance with a custom output bytes length
     pub fn with_output_bytes(output_bytes: usize) -> Self {
+        debug_assert!(
+            output_bytes <= Self::DEFAULT_BYTE_SIZE,
+            "GNU doesn't accept BLAKE2b bigger than 64 bytes long"
+        );
+
         let mut params = blake2b_simd::Params::new();
         params.hash_length(output_bytes);