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.
This commit is contained in:
Zhengchao An
2026-07-09 01:07:11 +08:00
committed by GitHub
parent 77eb91bf8d
commit afaf8c681a
2 changed files with 20 additions and 10 deletions
+1 -1
View File
@@ -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
)
}
+19 -9
View File
@@ -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::<Crc32>::default(),
Self::Crc32c => Box::<Crc32c>::default(),
Self::Crc64Nvme => Box::<Crc64Nvme>::default(),
#[allow(deprecated)]
Self::Md5 => Box::<Crc32>::default(),
Self::Sha1 => Box::<Sha1>::default(),
Self::Sha256 => Box::<Sha256>::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::<ChecksumAlgorithm>().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::<ChecksumAlgorithm>().expect_err("md5 should not parse");
assert_eq!("md5", error.checksum_algorithm());
let error = "MD5".parse::<ChecksumAlgorithm>().expect_err("md5 should not parse");
assert_eq!("MD5", error.checksum_algorithm());
}
}