diff --git a/crates/checksums/src/lib.rs b/crates/checksums/src/lib.rs index 5b566fe83..e3f397384 100644 --- a/crates/checksums/src/lib.rs +++ b/crates/checksums/src/lib.rs @@ -41,14 +41,18 @@ pub const XXHASH_64_NAME: &str = "xxhash64"; pub const XXHASH_128_NAME: &str = "xxhash128"; pub const MD5_NAME: &str = "md5"; -/// One of three deliberately separate checksum registries (backlog#1833): -/// this enum owns the **streaming-hash algorithm registry**, including the -/// RustFS extensions (sha512, xxhash3/64/128). The on-disk xl.meta bitset -/// lives in `rustfs_rio::ChecksumType` (crates/rio/src/checksum.rs, varint -/// bits are append-only), and the MinIO-port client keeps its own -/// `ChecksumMode` (crates/ecstore/src/client/checksum.rs). When adding an -/// algorithm, extend all three (or record why not) — they do not derive from -/// each other. +/// The canonical checksum-algorithm registry (backlog#1833, backlog#1844): +/// this enum owns the streaming-hash implementations and, via the exhaustive +/// per-algorithm metadata methods below, the wire names, header names, digest +/// lengths, and checksum-type capabilities — including the RustFS extensions +/// (sha512, xxhash3/64/128). The MinIO-port client's `ChecksumMode` +/// (crates/s3-client/src/checksum.rs) delegates all per-algorithm dispatch +/// here through its `algorithm()` bridge. The on-disk xl.meta bitset remains +/// deliberately separate in `rustfs_rio::ChecksumType` +/// (crates/rio/src/checksum.rs, varint bits are append-only). When adding an +/// algorithm: add the variant here (the exhaustive matches force every +/// metadata decision), bridge it in the client, and allocate an xl.meta bit +/// in rio (or record why not). #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] #[non_exhaustive] pub enum ChecksumAlgorithm { @@ -120,6 +124,84 @@ impl ChecksumAlgorithm { Self::Xxhash128 => XXHASH_128_NAME, } } + + // Per-algorithm wire metadata. These matches are deliberately exhaustive + // (no `_` arm): adding a ChecksumAlgorithm variant without deciding its + // name, header, digest length, and checksum-type support must fail to + // compile rather than silently inherit a default (backlog#1844). + + /// The canonical `x-amz-checksum-algorithm` wire value (uppercase), as + /// carried in S3 requests/responses and stored checksum maps. + pub fn s3_algorithm_name(&self) -> &'static str { + match self { + Self::Crc32 => "CRC32", + Self::Crc32c => "CRC32C", + Self::Crc64Nvme => "CRC64NVME", + Self::Sha1 => "SHA1", + Self::Sha256 => "SHA256", + Self::Sha512 => "SHA512", + Self::Xxhash3 => "XXHASH3", + Self::Xxhash64 => "XXHASH64", + Self::Xxhash128 => "XXHASH128", + } + } + + /// The `x-amz-checksum-*` HTTP header that carries this algorithm's + /// base64-encoded digest. + pub fn http_header_name(&self) -> &'static str { + match self { + Self::Crc32 => http::CRC_32_HEADER_NAME, + Self::Crc32c => http::CRC_32_C_HEADER_NAME, + Self::Crc64Nvme => http::CRC_64_NVME_HEADER_NAME, + Self::Sha1 => http::SHA_1_HEADER_NAME, + Self::Sha256 => http::SHA_256_HEADER_NAME, + Self::Sha512 => http::SHA_512_HEADER_NAME, + Self::Xxhash3 => http::XXHASH_3_HEADER_NAME, + Self::Xxhash64 => http::XXHASH_64_HEADER_NAME, + Self::Xxhash128 => http::XXHASH_128_HEADER_NAME, + } + } + + /// Raw (unencoded) digest length in bytes. + pub fn raw_len(&self) -> usize { + match self { + Self::Crc32 | Self::Crc32c => 4, + Self::Crc64Nvme => 8, + Self::Sha1 => 20, + Self::Sha256 => 32, + Self::Sha512 => 64, + Self::Xxhash3 | Self::Xxhash64 => 8, + Self::Xxhash128 => 16, + } + } + + /// Whether the algorithm supports the S3 COMPOSITE multipart checksum + /// type. Per the AWS registry, every algorithm does except CRC64NVME, + /// which is FULL_OBJECT-only. + pub fn supports_composite(&self) -> bool { + match self { + Self::Crc64Nvme => false, + Self::Crc32 + | Self::Crc32c + | Self::Sha1 + | Self::Sha256 + | Self::Sha512 + | Self::Xxhash3 + | Self::Xxhash64 + | Self::Xxhash128 => true, + } + } + + /// Whether the algorithm supports the S3 FULL_OBJECT checksum type, i.e. + /// part digests can be linearly combined into the whole-object digest. + /// Only the CRC family has this property; the hash algorithms are + /// COMPOSITE-only. + pub fn supports_full_object(&self) -> bool { + match self { + Self::Crc32 | Self::Crc32c | Self::Crc64Nvme => true, + Self::Sha1 | Self::Sha256 | Self::Sha512 | Self::Xxhash3 | Self::Xxhash64 | Self::Xxhash128 => false, + } + } } pub trait Checksum: Send + Sync { @@ -731,6 +813,77 @@ mod tests { assert_eq!(&raw[..], reference.digest128().to_be_bytes().as_slice()); } + #[test] + fn test_algorithm_metadata_is_consistent_for_every_variant() { + use crate::Checksum; + + // Cross-checks the per-algorithm metadata methods against the hasher + // implementations themselves, so the registry cannot drift from the + // code that computes digests (backlog#1844). The list must cover every + // variant; the metadata methods use exhaustive matches, so a new + // variant that is missing here still fails to compile there first. + let all = [ + ChecksumAlgorithm::Crc32, + ChecksumAlgorithm::Crc32c, + ChecksumAlgorithm::Crc64Nvme, + ChecksumAlgorithm::Sha1, + ChecksumAlgorithm::Sha256, + ChecksumAlgorithm::Sha512, + ChecksumAlgorithm::Xxhash3, + ChecksumAlgorithm::Xxhash64, + ChecksumAlgorithm::Xxhash128, + ]; + + for algorithm in all { + // Digest length must match what the hasher actually produces. + let mut hasher = algorithm.into_impl(); + hasher.update(b"metadata consistency probe"); + assert_eq!( + algorithm.raw_len(), + Checksum::size(&*algorithm.into_impl()) as usize, + "{algorithm:?} raw_len() != hasher size()" + ); + assert_eq!(hasher.finalize().len(), algorithm.raw_len(), "{algorithm:?} finalize length != raw_len()"); + + // Header name must match the hasher's own header binding. + assert_eq!( + algorithm.http_header_name(), + algorithm.into_impl().header_name(), + "{algorithm:?} http_header_name() != HttpChecksum::header_name()" + ); + assert_eq!( + algorithm.http_header_name(), + format!("x-amz-checksum-{}", algorithm.as_str()), + "{algorithm:?} header must be x-amz-checksum-" + ); + + // The uppercase wire name and the lowercase parse name must be the + // same word, and the wire name must parse back to the variant. + assert!( + algorithm.s3_algorithm_name().eq_ignore_ascii_case(algorithm.as_str()), + "{algorithm:?} s3_algorithm_name() and as_str() diverge" + ); + assert_eq!(algorithm.s3_algorithm_name().parse::().unwrap(), algorithm); + } + + // AWS checksum-type support table: CRC64NVME is FULL_OBJECT-only, the + // CRC family supports FULL_OBJECT, everything else is COMPOSITE-only. + for algorithm in all { + let composite = algorithm.supports_composite(); + let full_object = algorithm.supports_full_object(); + assert!(composite || full_object, "{algorithm:?} supports no checksum type at all"); + match algorithm { + ChecksumAlgorithm::Crc32 | ChecksumAlgorithm::Crc32c => { + assert!(composite && full_object, "{algorithm:?} must support both checksum types") + } + ChecksumAlgorithm::Crc64Nvme => { + assert!(!composite && full_object, "CRC64NVME must be FULL_OBJECT-only") + } + _ => assert!(composite && !full_object, "{algorithm:?} must be COMPOSITE-only"), + } + } + } + #[test] fn test_xxhash64_matches_direct_computation_big_endian_seed0() { use crate::Xxhash64; diff --git a/crates/s3-client/src/checksum.rs b/crates/s3-client/src/checksum.rs index 6b4de15d7..b85abb06b 100644 --- a/crates/s3-client/src/checksum.rs +++ b/crates/s3-client/src/checksum.rs @@ -19,31 +19,27 @@ #![allow(unused_must_use)] #![allow(clippy::all)] -use lazy_static::lazy_static; use rustfs_checksums::ChecksumAlgorithm; use std::collections::HashMap; use crate::utils::base64_decode; use crate::utils::base64_encode; use crate::{api_put_object::PutObjectOptions, api_s3_datatypes::ObjectPart}; -// s3s::header has no CRC64NVME constant yet; the canonical RustFS copy lives -// in rustfs-utils' headers module. -use rustfs_utils::http::headers::AMZ_CHECKSUM_CRC64NVME; -use s3s::header::{ - X_AMZ_CHECKSUM_ALGORITHM, X_AMZ_CHECKSUM_CRC32, X_AMZ_CHECKSUM_CRC32C, X_AMZ_CHECKSUM_SHA1, X_AMZ_CHECKSUM_SHA256, -}; -use enumset::{EnumSet, EnumSetType, enum_set}; +use enumset::EnumSetType; -/// One of three deliberately separate checksum registries (backlog#1833): -/// this enum is the MinIO-port client's wire vocabulary and stops at the -/// standard S3 set (CRC64NVME is its newest member; the RustFS extensions do -/// not exist on this client path). The streaming-hash registry lives in -/// `rustfs_checksums::ChecksumAlgorithm` (crates/checksums/src/lib.rs) and -/// the on-disk xl.meta bitset in `rustfs_rio::ChecksumType` -/// (crates/rio/src/checksum.rs, varint bits are append-only). When adding an -/// algorithm, extend all three (or record why not) — they do not derive from -/// each other. +/// The MinIO-port client's checksum vocabulary: the standard S3 algorithm set +/// plus the `None`/`FullObject` markers this client's option plumbing needs +/// (the RustFS extension algorithms do not exist on this client path). All +/// per-algorithm dispatch — header names, wire names, digest lengths, +/// checksum-type capabilities, hashers — is delegated through [`Self::algorithm`] +/// to the canonical registry in `rustfs_checksums::ChecksumAlgorithm` +/// (crates/checksums/src/lib.rs); only the variant-to-algorithm bridge lives +/// here. The on-disk xl.meta bitset remains separate in +/// `rustfs_rio::ChecksumType` (crates/rio/src/checksum.rs, varint bits are +/// append-only). When adding an algorithm: extend `ChecksumAlgorithm` (its +/// exhaustive matches force the metadata), add the variant + one `algorithm()` +/// arm here, and allocate an xl.meta bit in rio (or record why not). #[derive(Debug, EnumSetType, Default)] #[enumset(repr = "u8")] pub enum ChecksumMode { @@ -57,34 +53,31 @@ pub enum ChecksumMode { ChecksumFullObject, } -lazy_static! { - static ref C_ChecksumMask: EnumSet = { - let mut s = EnumSet::all(); - s.remove(ChecksumMode::ChecksumFullObject); - s - }; - static ref C_ChecksumFullObjectCRC32: EnumSet = - enum_set!(ChecksumMode::ChecksumCRC32 | ChecksumMode::ChecksumFullObject); - static ref C_ChecksumFullObjectCRC32C: EnumSet = - enum_set!(ChecksumMode::ChecksumCRC32C | ChecksumMode::ChecksumFullObject); -} impl ChecksumMode { - //pub const CRC64_NVME_POLYNOMIAL: i64 = 0xad93d23594c93659; + /// The single bridge from this client vocabulary to the canonical + /// algorithm registry. Every per-algorithm question below goes through + /// here; the marker variants (`ChecksumNone`, bare `ChecksumFullObject`) + /// map to `None` and fail closed at each call site. + pub fn algorithm(&self) -> Option { + match self { + ChecksumMode::ChecksumSHA256 => Some(ChecksumAlgorithm::Sha256), + ChecksumMode::ChecksumSHA1 => Some(ChecksumAlgorithm::Sha1), + ChecksumMode::ChecksumCRC32 => Some(ChecksumAlgorithm::Crc32), + ChecksumMode::ChecksumCRC32C => Some(ChecksumAlgorithm::Crc32c), + ChecksumMode::ChecksumCRC64NVME => Some(ChecksumAlgorithm::Crc64Nvme), + ChecksumMode::ChecksumNone | ChecksumMode::ChecksumFullObject => None, + } + } pub fn base(&self) -> ChecksumMode { - let s = EnumSet::from(*self).intersection(*C_ChecksumMask); - match s.as_u8() { - 1_u8 => ChecksumMode::ChecksumNone, - 2_u8 => ChecksumMode::ChecksumSHA256, - 4_u8 => ChecksumMode::ChecksumSHA1, - 8_u8 => ChecksumMode::ChecksumCRC32, - 16_u8 => ChecksumMode::ChecksumCRC32C, - 32_u8 => ChecksumMode::ChecksumCRC64NVME, - // Fail closed: any mode without a concrete base algorithm (e.g. a - // bare ChecksumFullObject flag) is treated as "no checksum" rather - // than panicking. Callers already gate real work behind - // is_set()/can_composite()/hasher(), so this only removes a crash. - _ => ChecksumMode::ChecksumNone, + // Fail closed: any mode without a concrete base algorithm (e.g. a + // bare ChecksumFullObject flag) is treated as "no checksum" rather + // than panicking. Callers already gate real work behind + // is_set()/can_composite()/hasher(), so this only removes a crash. + if self.algorithm().is_some() { + *self + } else { + ChecksumMode::ChecksumNone } } @@ -93,58 +86,22 @@ impl ChecksumMode { } pub fn key(&self) -> String { - //match c & checksumMask { - match self { - ChecksumMode::ChecksumCRC32 => { - return X_AMZ_CHECKSUM_CRC32.to_string(); - } - ChecksumMode::ChecksumCRC32C => { - return X_AMZ_CHECKSUM_CRC32C.to_string(); - } - ChecksumMode::ChecksumSHA1 => { - return X_AMZ_CHECKSUM_SHA1.to_string(); - } - ChecksumMode::ChecksumSHA256 => { - return X_AMZ_CHECKSUM_SHA256.to_string(); - } - ChecksumMode::ChecksumCRC64NVME => { - return AMZ_CHECKSUM_CRC64NVME.to_string(); - } - _ => { - return "".to_string(); - } - } + self.algorithm().map(|a| a.http_header_name().to_string()).unwrap_or_default() } pub fn can_composite(&self) -> bool { - let s = EnumSet::from(*self).intersection(*C_ChecksumMask); - match s.as_u8() { - 2_u8 => true, - 4_u8 => true, - 8_u8 => true, - 16_u8 => true, - _ => false, - } + self.algorithm().is_some_and(|a| a.supports_composite()) } pub fn can_merge_crc(&self) -> bool { - let s = EnumSet::from(*self).intersection(*C_ChecksumMask); - match s.as_u8() { - 8_u8 => true, - 16_u8 => true, - 32_u8 => true, - _ => false, - } + self.algorithm().is_some_and(|a| a.supports_full_object()) } pub fn full_object_requested(&self) -> bool { - let s = EnumSet::from(*self).intersection(*C_ChecksumMask); - match s.as_u8() { - //C_ChecksumFullObjectCRC32 as u8 => true, - //C_ChecksumFullObjectCRC32C as u8 => true, - 32_u8 => true, - _ => false, - } + // CRC64NVME is FULL_OBJECT-only, so selecting it implies a full-object + // checksum even without an explicit request (AWS behaviour). + self.algorithm() + .is_some_and(|a| a.supports_full_object() && !a.supports_composite()) } pub fn key_capitalized(&self) -> String { @@ -152,57 +109,23 @@ impl ChecksumMode { } pub fn raw_byte_len(&self) -> usize { - let u = EnumSet::from(*self).intersection(*C_ChecksumMask).as_u8(); - if u == ChecksumMode::ChecksumCRC32 as u8 || u == ChecksumMode::ChecksumCRC32C as u8 { - 4 - } else if u == ChecksumMode::ChecksumSHA1 as u8 { - use sha1::Digest; - sha1::Sha1::output_size() as usize - } else if u == ChecksumMode::ChecksumSHA256 as u8 { - use sha2::Digest; - sha2::Sha256::output_size() as usize - } else if u == ChecksumMode::ChecksumCRC64NVME as u8 { - 8 - } else { - 0 - } + self.algorithm().map(|a| a.raw_len()).unwrap_or(0) } pub fn hasher(&self) -> Result, std::io::Error> { - match /*C_ChecksumMask & **/self { - ChecksumMode::ChecksumCRC32 => { - return Ok(ChecksumAlgorithm::Crc32.into_impl()); - } - ChecksumMode::ChecksumCRC32C => { - return Ok(ChecksumAlgorithm::Crc32c.into_impl()); - } - ChecksumMode::ChecksumSHA1 => { - return Ok(ChecksumAlgorithm::Sha1.into_impl()); - } - ChecksumMode::ChecksumSHA256 => { - return Ok(ChecksumAlgorithm::Sha256.into_impl()); - } - ChecksumMode::ChecksumCRC64NVME => { - return Ok(ChecksumAlgorithm::Crc64Nvme.into_impl()); - } - _ => return Err(std::io::Error::other("unsupported checksum type")), - } + self.algorithm() + .map(ChecksumAlgorithm::into_impl) + .ok_or_else(|| std::io::Error::other("unsupported checksum type")) } pub fn is_set(&self) -> bool { - // `ChecksumNone` is the zeroth enum variant, so it occupies bit 0 of the - // `EnumSet` repr and a naive `len() == 1` check reports "no checksum" as a - // configured checksum. A checksum is only "set" when a concrete algorithm - // (one with a real hasher) is selected; the bare `ChecksumFullObject` flag - // has no base algorithm and is likewise not set. Treating `ChecksumNone` - // as set made ILM transitions of >128 MiB objects fail with - // "unsupported checksum type" (rustfs/rustfs#4811): the multipart put path - // took the checksum branch and called `ChecksumNone.hasher()`. - if matches!(self, ChecksumMode::ChecksumNone) { - return false; - } - let s = EnumSet::from(*self).intersection(*C_ChecksumMask); - s.len() == 1 + // A checksum is only "set" when a concrete algorithm (one with a real + // hasher) is selected; `ChecksumNone` and the bare `ChecksumFullObject` + // flag are not. Treating `ChecksumNone` as set made ILM transitions of + // >128 MiB objects fail with "unsupported checksum type" + // (rustfs/rustfs#4811): the multipart put path took the checksum branch + // and called `ChecksumNone.hasher()`. + self.algorithm().is_some() } pub fn set_default(&mut self, t: ChecksumMode) { @@ -222,29 +145,10 @@ impl ChecksumMode { } pub fn to_string(&self) -> String { - //match c & checksumMask { - match self { - ChecksumMode::ChecksumCRC32 => { - return "CRC32".to_string(); - } - ChecksumMode::ChecksumCRC32C => { - return "CRC32C".to_string(); - } - ChecksumMode::ChecksumSHA1 => { - return "SHA1".to_string(); - } - ChecksumMode::ChecksumSHA256 => { - return "SHA256".to_string(); - } - ChecksumMode::ChecksumNone => { - return "".to_string(); - } - ChecksumMode::ChecksumCRC64NVME => { - return "CRC64NVME".to_string(); - } - _ => { - return "".to_string(); - } + match self.algorithm() { + Some(algorithm) => algorithm.s3_algorithm_name().to_string(), + None if matches!(self, ChecksumMode::ChecksumNone) => "".to_string(), + None => "".to_string(), } } @@ -344,6 +248,109 @@ mod tests { } } + #[test] + fn test_delegated_dispatch_preserves_wire_behaviour() { + // Behaviour lock for the backlog#1844 unification: every output that + // reaches the wire (header names, algorithm names, digest lengths, + // checksum-type capabilities) is pinned to the exact values the + // pre-delegation per-algorithm matches produced. If delegation to + // rustfs_checksums::ChecksumAlgorithm ever drifts, this fails loudly. + struct Expected { + mode: ChecksumMode, + key: &'static str, + name: &'static str, + raw_len: usize, + composite: bool, + merge_crc: bool, + full_object: bool, + } + let table = [ + Expected { + mode: ChecksumMode::ChecksumCRC32, + key: "x-amz-checksum-crc32", + name: "CRC32", + raw_len: 4, + composite: true, + merge_crc: true, + full_object: false, + }, + Expected { + mode: ChecksumMode::ChecksumCRC32C, + key: "x-amz-checksum-crc32c", + name: "CRC32C", + raw_len: 4, + composite: true, + merge_crc: true, + full_object: false, + }, + Expected { + mode: ChecksumMode::ChecksumSHA1, + key: "x-amz-checksum-sha1", + name: "SHA1", + raw_len: 20, + composite: true, + merge_crc: false, + full_object: false, + }, + Expected { + mode: ChecksumMode::ChecksumSHA256, + key: "x-amz-checksum-sha256", + name: "SHA256", + raw_len: 32, + composite: true, + merge_crc: false, + full_object: false, + }, + Expected { + mode: ChecksumMode::ChecksumCRC64NVME, + key: "x-amz-checksum-crc64nvme", + name: "CRC64NVME", + raw_len: 8, + composite: false, + merge_crc: true, + full_object: true, + }, + Expected { + mode: ChecksumMode::ChecksumNone, + key: "", + name: "", + raw_len: 0, + composite: false, + merge_crc: false, + full_object: false, + }, + Expected { + mode: ChecksumMode::ChecksumFullObject, + key: "", + name: "", + raw_len: 0, + composite: false, + merge_crc: false, + full_object: false, + }, + ]; + + for e in table { + assert_eq!(e.mode.key(), e.key, "{:?} key()", e.mode); + assert_eq!(e.mode.key_capitalized(), e.key, "{:?} key_capitalized()", e.mode); + assert_eq!(e.mode.to_string(), e.name, "{:?} to_string()", e.mode); + assert_eq!(e.mode.raw_byte_len(), e.raw_len, "{:?} raw_byte_len()", e.mode); + assert_eq!(e.mode.can_composite(), e.composite, "{:?} can_composite()", e.mode); + assert_eq!(e.mode.can_merge_crc(), e.merge_crc, "{:?} can_merge_crc()", e.mode); + assert_eq!(e.mode.full_object_requested(), e.full_object, "{:?} full_object_requested()", e.mode); + + // The hasher, when present, must produce digests of the advertised + // length under the advertised header name. + if let Ok(mut hasher) = e.mode.hasher() { + assert_eq!(e.mode.hasher().unwrap().header_name(), e.key, "{:?} hasher header", e.mode); + hasher.update(b"wire behaviour probe"); + assert_eq!(hasher.finalize().len(), e.raw_len, "{:?} digest length", e.mode); + } else { + assert_eq!(e.raw_len, 0, "{:?} has no hasher but a nonzero raw_len", e.mode); + } + } + } + #[test] fn test_set_default_upgrades_none() { // With `is_set()` fixed, `set_default` must upgrade an unset mode to the