From afaf8c681ab89806bc0393e5993da81810533e94 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 9 Jul 2026 01:07:11 +0800 Subject: [PATCH] fix(checksums): reject md5 instead of silently returning crc32 (#4513) The UnknownChecksumAlgorithmError message omitted crc64nvme and advertised the deprecated md5. Parsing "md5" also returned a Crc32 algorithm and its into_impl produced a 4-byte CRC32, so a caller asking for MD5 silently got the wrong digest. Enumerate the accepted algorithms (crc32, crc32c, crc64nvme, sha1, sha256) in the error message, remove the unused Md5 enum variant, and make parsing "md5" fail loudly. Add tests covering the message contents and the md5 parse error. --- crates/checksums/src/error.rs | 2 +- crates/checksums/src/lib.rs | 28 +++++++++++++++++++--------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/crates/checksums/src/error.rs b/crates/checksums/src/error.rs index fcdb6d8d6..9749e9739 100644 --- a/crates/checksums/src/error.rs +++ b/crates/checksums/src/error.rs @@ -36,7 +36,7 @@ impl fmt::Display for UnknownChecksumAlgorithmError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - r#"unknown checksum algorithm "{}", please pass a known algorithm name ("crc32", "crc32c", "sha1", "sha256", "md5")"#, + r#"unknown checksum algorithm "{}", please pass a known algorithm name ("crc32", "crc32c", "crc64nvme", "sha1", "sha256")"#, self.checksum_algorithm ) } diff --git a/crates/checksums/src/lib.rs b/crates/checksums/src/lib.rs index c93c59798..0a3dfb423 100644 --- a/crates/checksums/src/lib.rs +++ b/crates/checksums/src/lib.rs @@ -43,8 +43,6 @@ pub enum ChecksumAlgorithm { #[default] Crc32, Crc32c, - #[deprecated] - Md5, Sha1, Sha256, Crc64Nvme, @@ -62,9 +60,6 @@ impl FromStr for ChecksumAlgorithm { Ok(Self::Sha1) } else if checksum_algorithm.eq_ignore_ascii_case(SHA_256_NAME) { Ok(Self::Sha256) - } else if checksum_algorithm.eq_ignore_ascii_case(MD5_NAME) { - // MD5 is now an alias for the default Crc32 since it is deprecated - Ok(Self::Crc32) } else if checksum_algorithm.eq_ignore_ascii_case(CRC_64_NVME_NAME) { Ok(Self::Crc64Nvme) } else { @@ -79,8 +74,6 @@ impl ChecksumAlgorithm { Self::Crc32 => Box::::default(), Self::Crc32c => Box::::default(), Self::Crc64Nvme => Box::::default(), - #[allow(deprecated)] - Self::Md5 => Box::::default(), Self::Sha1 => Box::::default(), Self::Sha256 => Box::::default(), } @@ -91,8 +84,6 @@ impl ChecksumAlgorithm { Self::Crc32 => CRC_32_NAME, Self::Crc32c => CRC_32_C_NAME, Self::Crc64Nvme => CRC_64_NVME_NAME, - #[allow(deprecated)] - Self::Md5 => MD5_NAME, Self::Sha1 => SHA_1_NAME, Self::Sha256 => SHA_256_NAME, } @@ -444,4 +435,23 @@ mod tests { .expect_err("it should error"); assert_eq!("some invalid checksum algorithm", error.checksum_algorithm()); } + + #[test] + fn test_unknown_algorithm_error_message_lists_supported_algorithms() { + let error = "nope".parse::().expect_err("it should error"); + let message = error.to_string(); + assert!(message.contains("crc64nvme"), "message should advertise crc64nvme: {message}"); + assert!(!message.contains("md5"), "message should not advertise the unsupported md5: {message}"); + } + + #[test] + fn test_md5_is_not_a_supported_checksum_algorithm() { + // MD5 is not an accepted S3 checksum algorithm here: parsing it must fail + // loudly rather than silently substituting a CRC32 hasher. + let error = "md5".parse::().expect_err("md5 should not parse"); + assert_eq!("md5", error.checksum_algorithm()); + + let error = "MD5".parse::().expect_err("md5 should not parse"); + assert_eq!("MD5", error.checksum_algorithm()); + } }