From be6859be553c292a98b04b83148ed19ec20e0f9f Mon Sep 17 00:00:00 2001 From: houseme Date: Wed, 15 Jul 2026 14:31:37 +0800 Subject: [PATCH] fix(ecstore): handle ChecksumNone in >128 MiB ILM transitions (#4831) * fix(ecstore): treat ChecksumNone as unset so >128 MiB ILM transitions succeed ILM transition of any object larger than 128 MiB to a RustFS-native tier (rustfs/minio/aliyun/tencent/r2/azure/huaweicloud/s3 backends that use the built-in TransitionClient) failed with "unsupported checksum type", while objects <=128 MiB transitioned fine. Root cause: `ChecksumMode::is_set()` reported `ChecksumNone` as a configured checksum. `ChecksumNone` is the zeroth enum variant, so it occupies bit 0 of the EnumSet repr and the `len() == 1` check treated "no checksum" as set. The 128 MiB boundary is the warm backend's `MIN_PART_SIZE`, which selects a single PUT (<=128 MiB) versus a multipart PUT (>128 MiB). On the multipart path, `put_object_multipart_stream_optional_checksum` saw `checksum.is_set() == true`, disabled the Content-MD5 branch, and called `ChecksumNone.hasher()`, which returns the "unsupported checksum type" error. The single-PUT path hit the same misjudgement but never calls `hasher()`, so it silently succeeded (without a checksum), which is why only >128 MiB objects failed. Fix: - `is_set()` returns false for `ChecksumNone` (and the bare `ChecksumFullObject` flag, which has no base algorithm). This is the sole callers' intended meaning: a concrete algorithm with a real hasher is selected. - Defense in depth: guard the multipart checksum branch on `auto_checksum.is_set()` so an unset mode uploads the part without a per-part checksum header instead of hard-failing in `hasher()`. Only the TransitionClient consumes this `ChecksumMode::is_set()`; the server-side data path uses the unrelated `rustfs_rio::ChecksumType`. Tests: is_set()/set_default semantics, hasher parity for every set mode, and a `build_transition_put_options` invariant (checksum unset + Content-MD5 on). Refs: rustfs/rustfs#4811, rustfs/backlog#1267 Co-Authored-By: heihutu * fix(ecstore): read exactly one part per multipart chunk in transition uploads Second defect behind the >128 MiB ILM transition failure (rustfs/rustfs#4811), uncovered while verifying the checksum fix. `put_object_multipart_stream_optional_checksum` read each part with `read_all()` / `to_vec()`, which drained the entire source into the first part and left every later part empty. Any multipart upload of a streamed (`ObjectBody`) source was therefore malformed. Objects <=128 MiB take the single-part path and were unaffected; a 128 MiB + 1 byte object splits into a 128 MiB part plus a 1 byte part, so the first part received the whole object and its declared Content-Length (part_size) did not match the body. Verified empirically: `optimal_part_info(128 MiB + 1, 128 MiB)` yields 2 parts, and `GetObjectReader::read_all()` on part 1 returns the full 134217729 bytes, leaving 0 for part 2. Fix: - Add `read_multipart_part`, which reads exactly the requested part size (or less at EOF) and advances the reader, for both `Body` (in-memory) and `ObjectBody` (streamed) sources. - Upload each part with the bytes actually read (`length`) as its size, and account uploaded size by actual bytes, so a short read is detected instead of masked. The concurrent (`put_object_multipart_stream_parallel`) and SigV2 (`put_object_multipart`) paths share the same `read_all()` pattern but are not exercised by transition; left untouched here and noted for follow-up. Tests: `read_multipart_part` splits a 250-byte source into [100, 100, 50] for both streamed and in-memory bodies, consumes the source fully, and stops at EOF without overrun. Refs: rustfs/rustfs#4811, rustfs/backlog#1267 Co-Authored-By: heihutu * fix(ecstore): complete the >128 MiB ILM transition multipart client Docker end-to-end reproduction of rustfs/rustfs#4811 (two RustFS tiers, a 128 MiB + 1 byte object, zero-day transition) surfaced four more defects on the multipart transition path, each masked by the previous one. With the checksum and part-splitting fixes in place the transition now failed later and later, and finally produced a 0-byte object with no error at all. Fixed together: - initiate_multipart_upload discarded the CreateMultipartUpload response and returned an empty UploadId, so the first UploadPart failed with "UploadID cannot be empty". Parse the response XML (InitiateMultipartUploadResult now derives Deserialize with PascalCase). - Content-MD5 / x-amz-checksum-* were encoded with URL-safe, unpadded base64, which the remote rejected as "Invalid content MD5: Base64Error". Add base64_encode_standard and use it for those outbound header values. - PutObjectOptions::default() set legalhold to OFF, so header() attached x-amz-object-lock-legal-hold to every request and CompleteMultipartUpload was rejected with "does not accept object lock or governance bypass headers". Default to an empty (unset) status. - CompleteMultipartUpload / CompletePart had no serde renames, so the request body used Rust field names (//). The remote parsed zero elements and completed a 0-byte object while returning 200. Emit S3 element names (//) and skip empty checksum fields. Verified end-to-end: a 128 MiB + 1 byte object now transitions to the remote tier and reads back (transparently restored) byte-for-byte identical (sha256 match), with none of the four prior errors in the logs. Refs: rustfs/rustfs#4811, rustfs/backlog#1267 Co-Authored-By: heihutu --------- Co-authored-by: heihutu --- crates/ecstore/src/client/api_put_object.rs | 6 +- .../src/client/api_put_object_multipart.rs | 72 +++++++++- .../src/client/api_put_object_streaming.rs | 134 ++++++++++++++++-- crates/ecstore/src/client/api_s3_datatypes.rs | 20 ++- crates/ecstore/src/client/checksum.rs | 48 +++++++ crates/ecstore/src/client/utils.rs | 33 +++++ .../ecstore/src/services/tier/warm_backend.rs | 15 ++ 7 files changed, 309 insertions(+), 19 deletions(-) diff --git a/crates/ecstore/src/client/api_put_object.rs b/crates/ecstore/src/client/api_put_object.rs index 4d661bee9..cb5c98e97 100644 --- a/crates/ecstore/src/client/api_put_object.rs +++ b/crates/ecstore/src/client/api_put_object.rs @@ -120,7 +120,11 @@ impl Default for PutObjectOptions { storage_class: "".to_string(), website_redirect_location: "".to_string(), part_size: 0, - legalhold: ObjectLockLegalHoldStatus::from_static(ObjectLockLegalHoldStatus::OFF), + // Empty, not OFF: `header()` emits x-amz-object-lock-legal-hold for + // any non-empty status, and CompleteMultipartUpload rejects requests + // that carry object-lock headers, breaking multipart transitions + // (rustfs/rustfs#4811). Only send the header when a status is set. + legalhold: ObjectLockLegalHoldStatus::from_static(""), send_content_md5: false, disable_content_sha256: false, disable_multipart: false, diff --git a/crates/ecstore/src/client/api_put_object_multipart.rs b/crates/ecstore/src/client/api_put_object_multipart.rs index 66c621016..8597b62b0 100644 --- a/crates/ecstore/src/client/api_put_object_multipart.rs +++ b/crates/ecstore/src/client/api_put_object_multipart.rs @@ -18,6 +18,7 @@ #![allow(clippy::all)] use http::{HeaderMap, HeaderName, StatusCode}; +use http_body_util::BodyExt; use hyper::body::Bytes; use s3s::S3ErrorCode; use std::collections::HashMap; @@ -244,7 +245,23 @@ impl TransitionClient { ))); } //} - let initiate_multipart_upload_result = InitiateMultipartUploadResult::default(); + // Parse the CreateMultipartUpload response for the UploadId. Returning a + // default (empty) result here made every multipart transition fail at the + // first UploadPart with "UploadID cannot be empty" (rustfs/rustfs#4811). + let mut body_vec = Vec::new(); + let mut body = resp.into_body(); + while let Some(frame) = body.frame().await { + let frame = frame.map_err(|e| std::io::Error::other(e.to_string()))?; + if let Some(data) = frame.data_ref() { + body_vec.extend_from_slice(data); + } + } + let initiate_multipart_upload_result = + quick_xml::de::from_str::(&String::from_utf8_lossy(&body_vec)) + .map_err(|e| std::io::Error::other(format!("failed to parse CreateMultipartUpload response: {e}")))?; + if initiate_multipart_upload_result.upload_id.is_empty() { + return Err(std::io::Error::other("CreateMultipartUpload response missing UploadId")); + } Ok(initiate_multipart_upload_result) } @@ -432,3 +449,56 @@ pub struct UploadPartParams { pub custom_header: HeaderMap, pub trailer: HeaderMap, } + +#[cfg(test)] +mod tests { + use crate::client::api_s3_datatypes::{CompleteMultipartUpload, CompletePart, InitiateMultipartUploadResult}; + + #[test] + fn complete_multipart_upload_serializes_s3_part_elements() { + // Regression for rustfs/rustfs#4811: without serde renames quick-xml emits + // //, so the remote parses zero elements and + // completes a 0-byte object (while still returning 200). The body must use + // S3 element names, and MD5-only transitions must not emit empty checksum + // elements. + let complete = CompleteMultipartUpload { + parts: vec![ + CompletePart { + part_num: 1, + etag: "etag-one".to_string(), + ..Default::default() + }, + CompletePart { + part_num: 2, + etag: "etag-two".to_string(), + ..Default::default() + }, + ], + }; + let xml = complete.marshal_msg().expect("marshal"); + assert!(xml.contains(""), "missing : {xml}"); + assert!(xml.contains("1"), "missing PartNumber: {xml}"); + assert!(xml.contains("etag-one"), "missing ETag: {xml}"); + assert!(xml.contains("2"), "missing part 2: {xml}"); + assert!(!xml.contains("part_num"), "leaked rust field name: {xml}"); + assert!(!xml.contains(""), "emitted empty checksum: {xml}"); + } + + #[test] + fn parses_create_multipart_upload_response() { + // Regression for rustfs/rustfs#4811: `initiate_multipart_upload` used to + // return a default (empty) result, so every multipart transition failed + // the first UploadPart with "UploadID cannot be empty". The UploadId must + // be parsed out of the S3 XML response. + let xml = r#" + + bar + foo/payload.bin + a1b2c3-d4e5-f6 +"#; + let parsed: InitiateMultipartUploadResult = quick_xml::de::from_str(xml).expect("parse"); + assert_eq!(parsed.upload_id, "a1b2c3-d4e5-f6"); + assert_eq!(parsed.bucket, "bar"); + assert_eq!(parsed.key, "foo/payload.bin"); + } +} diff --git a/crates/ecstore/src/client/api_put_object_streaming.rs b/crates/ecstore/src/client/api_put_object_streaming.rs index 67df7e42b..c640bc0c9 100644 --- a/crates/ecstore/src/client/api_put_object_streaming.rs +++ b/crates/ecstore/src/client/api_put_object_streaming.rs @@ -25,6 +25,7 @@ use std::io::Error; use std::sync::RwLock; use std::{collections::HashMap, sync::Arc}; use time::{OffsetDateTime, format_description}; +use tokio::io::AsyncReadExt; use tokio::{select, sync::mpsc}; use tokio_util::sync::CancellationToken; use tracing::warn; @@ -41,10 +42,37 @@ use crate::client::{ transition_api::{ReaderImpl, RequestMetadata, TransitionClient, UploadInfo}, }; -use crate::client::utils::base64_encode; +use crate::client::utils::{base64_encode, base64_encode_standard}; use rustfs_utils::path::trim_etag; use s3s::header::{X_AMZ_EXPIRATION, X_AMZ_VERSION_ID}; +/// Read exactly `want` bytes for a single multipart part, or fewer if the reader +/// reaches EOF first. Advances the reader so the next call returns the following +/// part. Replaces the previous per-part `read_all()`/`to_vec()`, which drained +/// the entire source into the first part and left later parts empty +/// (rustfs/rustfs#4811). +async fn read_multipart_part(reader: &mut ReaderImpl, want: usize) -> Result, std::io::Error> { + match reader { + ReaderImpl::Body(content_body) => { + let take = content_body.len().min(want); + Ok(content_body.split_to(take).to_vec()) + } + ReaderImpl::ObjectBody(content_body) => { + let mut buf = vec![0u8; want]; + let mut filled = 0; + while filled < want { + let n = content_body.read(&mut buf[filled..]).await?; + if n == 0 { + break; + } + filled += n; + } + buf.truncate(filled); + Ok(buf) + } + } +} + pub struct UploadedPartRes { pub error: std::io::Error, pub part_num: i64, @@ -141,14 +169,12 @@ impl TransitionClient { part_size = lastpart_size; } - match &mut reader { - ReaderImpl::Body(content_body) => { - buf = content_body.to_vec(); - } - ReaderImpl::ObjectBody(content_body) => { - buf = content_body.read_all().await?; - } - } + // Read exactly this part's bytes. Using `read_all()`/`to_vec()` here + // drained the whole source into the first part and left every later + // part empty, silently corrupting any multipart upload of a streamed + // (`ObjectBody`) source — e.g. ILM transitions of >128 MiB objects, + // which split into 128 MiB parts (rustfs/rustfs#4811). + buf = read_multipart_part(&mut reader, part_size as usize).await?; let length = buf.len(); if opts.send_content_md5 { @@ -158,14 +184,16 @@ impl TransitionClient { None => return Err(std::io::Error::other("MD5 hasher not initialized")), }; let hash = md5_hash.hash_encode(&buf[..length]); - md5_base64 = base64_encode(hash.as_ref()); - } else { + // Content-MD5 must be standard base64 for the remote to accept it. + md5_base64 = base64_encode_standard(hash.as_ref()); + } else if opts.auto_checksum.is_set() { let mut crc = opts.auto_checksum.hasher()?; crc.update(&buf[..length]); let csum = crc.finalize(); if let Ok(header_name) = HeaderName::from_bytes(opts.auto_checksum.key().as_bytes()) { - if let Ok(header_value) = base64_encode(csum.as_ref()).parse() { + // x-amz-checksum-* header values are standard base64 too. + if let Ok(header_value) = base64_encode_standard(csum.as_ref()).parse() { custom_header.insert(header_name, header_value); } else { warn!("Failed to parse checksum value"); @@ -174,6 +202,10 @@ impl TransitionClient { warn!("Invalid header name: {}", opts.auto_checksum.key()); } } + // else: neither MD5 nor a concrete additional checksum was requested, + // so upload the part without a per-part checksum header. Guarding the + // branch on `is_set()` avoids calling `hasher()` on `ChecksumNone`, + // which errors with "unsupported checksum type" (rustfs/rustfs#4811). let hooked = ReaderImpl::Body(Bytes::from(buf)); //newHook(BufferReader::new(buf), opts.progress); let mut p = UploadPartParams { @@ -183,7 +215,9 @@ impl TransitionClient { reader: hooked, part_number, md5_base64: md5_base64.clone(), - size: part_size, + // Use the bytes actually read, not the planned part_size, so the + // uploaded Content-Length matches the body even on a short read. + size: length as i64, //sse: opts.server_side_encryption, stream_sha256: !opts.disable_content_sha256, custom_header: custom_header.clone(), @@ -194,7 +228,7 @@ impl TransitionClient { parts_info.entry(part_number).or_insert(obj_part); - total_uploaded_size += part_size as i64; + total_uploaded_size += length as i64; } if size > 0 && total_uploaded_size != size { @@ -593,9 +627,79 @@ fn collect_complete_parts(parts_info: &HashMap, total_parts_cou #[cfg(test)] mod tests { - use super::{ObjectPart, collect_complete_parts}; + use super::{ObjectPart, ReaderImpl, collect_complete_parts, read_multipart_part}; + use crate::object_api::GetObjectReader; + use bytes::Bytes; use std::collections::HashMap; + // Drive a reader through the same per-part loop the multipart stream uses and + // collect the size of every part. Regression for rustfs/rustfs#4811: the old + // `read_all()` per part drained the whole source into part 1. + async fn collect_part_sizes(mut reader: ReaderImpl, total: usize, part_size: usize, last_part_size: usize) -> Vec { + let parts = total.div_ceil(part_size); + let mut sizes = Vec::new(); + for part_number in 1..=parts { + let want = if part_number == parts { last_part_size } else { part_size }; + let buf = read_multipart_part(&mut reader, want).await.unwrap(); + sizes.push(buf.len()); + } + // Nothing must remain after the planned parts are consumed. + assert!(read_multipart_part(&mut reader, part_size).await.unwrap().is_empty()); + sizes + } + + #[tokio::test] + async fn read_multipart_part_splits_streamed_object_body_evenly() { + // 250 bytes at part_size 100 -> parts [100, 100, 50], mirroring the + // >128 MiB / 128 MiB split from the bug report on a small deterministic + // stream. + let total = 250usize; + let (mut w, r) = tokio::io::duplex(64); + tokio::spawn(async move { + use tokio::io::AsyncWriteExt; + let data: Vec = (0..total).map(|i| i as u8).collect(); + w.write_all(&data).await.unwrap(); + }); + let reader = ReaderImpl::ObjectBody(GetObjectReader { + stream: Box::new(r), + object_info: Default::default(), + buffered_body: None, + body_source: Default::default(), + }); + + let sizes = collect_part_sizes(reader, total, 100, 50).await; + assert_eq!(sizes, vec![100, 100, 50]); + } + + #[tokio::test] + async fn read_multipart_part_splits_in_memory_body_evenly() { + let total = 250usize; + let data: Vec = (0..total).map(|i| i as u8).collect(); + let reader = ReaderImpl::Body(Bytes::from(data)); + + let sizes = collect_part_sizes(reader, total, 100, 50).await; + assert_eq!(sizes, vec![100, 100, 50]); + } + + #[tokio::test] + async fn read_multipart_part_stops_at_eof_without_overrun() { + // Reader shorter than the requested part size must return only what is + // available, not block or pad. + let (mut w, r) = tokio::io::duplex(64); + tokio::spawn(async move { + use tokio::io::AsyncWriteExt; + w.write_all(&[1u8; 30]).await.unwrap(); + }); + let mut reader = ReaderImpl::ObjectBody(GetObjectReader { + stream: Box::new(r), + object_info: Default::default(), + buffered_body: None, + body_source: Default::default(), + }); + let buf = read_multipart_part(&mut reader, 100).await.unwrap(); + assert_eq!(buf.len(), 30); + } + fn parts_map(n: i64) -> HashMap { let mut m = HashMap::new(); for i in 1..=n { diff --git a/crates/ecstore/src/client/api_s3_datatypes.rs b/crates/ecstore/src/client/api_s3_datatypes.rs index 2dee0810f..dea2918a7 100644 --- a/crates/ecstore/src/client/api_s3_datatypes.rs +++ b/crates/ecstore/src/client/api_s3_datatypes.rs @@ -209,7 +209,8 @@ pub struct ListObjectPartsResult { pub encoding_type: String, } -#[derive(Debug, Default)] +#[derive(Debug, Default, Serialize, Deserialize)] +#[serde(default, rename_all = "PascalCase")] pub struct InitiateMultipartUploadResult { pub bucket: String, pub key: String, @@ -229,15 +230,28 @@ pub struct CompleteMultipartUploadResult { pub checksum_crc64nvme: String, } +// Field renames drive the CompleteMultipartUpload XML body sent to the remote. +// Without them quick-xml emits the Rust field names (/), so the +// remote parses zero elements and completes a 0-byte object while still +// returning 200 — the multipart transition silently produces an empty object +// (rustfs/rustfs#4811). Empty checksum fields are skipped so an MD5-only +// transition does not emit blank elements. #[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, serde::Serialize)] pub struct CompletePart { //api has - pub etag: String, + #[serde(rename = "PartNumber")] pub part_num: i64, + #[serde(rename = "ETag")] + pub etag: String, + #[serde(rename = "ChecksumCRC32", skip_serializing_if = "String::is_empty")] pub checksum_crc32: String, + #[serde(rename = "ChecksumCRC32C", skip_serializing_if = "String::is_empty")] pub checksum_crc32c: String, + #[serde(rename = "ChecksumSHA1", skip_serializing_if = "String::is_empty")] pub checksum_sha1: String, + #[serde(rename = "ChecksumSHA256", skip_serializing_if = "String::is_empty")] pub checksum_sha256: String, + #[serde(rename = "ChecksumCRC64NVME", skip_serializing_if = "String::is_empty")] pub checksum_crc64nvme: String, } @@ -272,7 +286,9 @@ pub struct CopyObjectPartResult { } #[derive(Debug, Default, serde::Serialize)] +#[serde(rename = "CompleteMultipartUpload")] pub struct CompleteMultipartUpload { + #[serde(rename = "Part")] pub parts: Vec, } diff --git a/crates/ecstore/src/client/checksum.rs b/crates/ecstore/src/client/checksum.rs index 228e7e16f..4d845fd66 100644 --- a/crates/ecstore/src/client/checksum.rs +++ b/crates/ecstore/src/client/checksum.rs @@ -181,6 +181,17 @@ impl ChecksumMode { } 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 } @@ -301,6 +312,43 @@ mod tests { assert!(ChecksumMode::ChecksumFullObject.hasher().is_err()); assert!(ChecksumMode::ChecksumCRC32.hasher().is_ok()); } + + #[test] + fn test_is_set_is_false_for_none_and_bare_full_object() { + // Regression for rustfs/rustfs#4811: `ChecksumNone` must NOT be reported as + // a configured checksum. It is the zeroth enum variant (bit 0 of the + // EnumSet repr), so the old `len() == 1` check treated it as set and drove + // the multipart put path into `ChecksumNone.hasher()` → "unsupported + // checksum type". Every mode reported as set must also have a real hasher. + assert!(!ChecksumMode::ChecksumNone.is_set()); + assert!(!ChecksumMode::ChecksumFullObject.is_set()); + + for mode in [ + ChecksumMode::ChecksumCRC32, + ChecksumMode::ChecksumCRC32C, + ChecksumMode::ChecksumSHA1, + ChecksumMode::ChecksumSHA256, + ChecksumMode::ChecksumCRC64NVME, + ] { + assert!(mode.is_set(), "{mode:?} should be set"); + assert!(mode.hasher().is_ok(), "{mode:?} reported set but has no hasher"); + } + } + + #[test] + fn test_set_default_upgrades_none() { + // With `is_set()` fixed, `set_default` must upgrade an unset mode to the + // provided default (previously `ChecksumNone` was seen as set and never + // upgraded). + let mut mode = ChecksumMode::ChecksumNone; + mode.set_default(ChecksumMode::ChecksumCRC32C); + assert_eq!(mode, ChecksumMode::ChecksumCRC32C); + + // An already-set mode is left untouched. + let mut existing = ChecksumMode::ChecksumSHA256; + existing.set_default(ChecksumMode::ChecksumCRC32C); + assert_eq!(existing, ChecksumMode::ChecksumSHA256); + } } #[derive(Default)] diff --git a/crates/ecstore/src/client/utils.rs b/crates/ecstore/src/client/utils.rs index 234783abc..a1e463087 100644 --- a/crates/ecstore/src/client/utils.rs +++ b/crates/ecstore/src/client/utils.rs @@ -95,6 +95,39 @@ pub fn base64_encode(input: &[u8]) -> String { base64_simd::URL_SAFE_NO_PAD.encode_to_string(input) } +/// Standard base64 (with `+`/`/` and `=` padding), as required by S3 for the +/// `Content-MD5` and `x-amz-checksum-*` request headers. The URL-safe, unpadded +/// [`base64_encode`] is only valid for internal round-trips; sending it as +/// `Content-MD5` makes the remote reject the part with "Invalid content MD5: +/// Base64Error" (rustfs/rustfs#4811). +pub fn base64_encode_standard(input: &[u8]) -> String { + base64_simd::STANDARD.encode_to_string(input) +} + pub fn base64_decode(input: &[u8]) -> Result, base64_simd::Error> { base64_simd::URL_SAFE_NO_PAD.decode_to_vec(input) } + +#[cfg(test)] +mod tests { + use super::base64_encode_standard; + + #[test] + fn base64_encode_standard_matches_s3_content_md5() { + // Standard base64 uses '+'/'/' and '=' padding and must round-trip + // through a standard decoder (which is what an S3 server uses to read + // Content-MD5). Regression for rustfs/rustfs#4811 ("Invalid content MD5: + // Base64Error"): the URL-safe, unpadded encoder produced values the + // remote could not decode. + // 16-byte MD5-length input chosen to force '=' padding. + let digest: [u8; 16] = [ + 0xfb, 0xff, 0xff, 0xef, 0x10, 0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80, 0x90, 0xa0, 0xb0, 0xc0, + ]; + let encoded = base64_encode_standard(&digest); + assert!(encoded.ends_with('='), "16-byte input must be padded: {encoded}"); + let decoded = base64_simd::STANDARD + .decode_to_vec(encoded.as_bytes()) + .expect("standard decode"); + assert_eq!(decoded, digest); + } +} diff --git a/crates/ecstore/src/services/tier/warm_backend.rs b/crates/ecstore/src/services/tier/warm_backend.rs index 0bbec9c0b..0dccc6f0b 100644 --- a/crates/ecstore/src/services/tier/warm_backend.rs +++ b/crates/ecstore/src/services/tier/warm_backend.rs @@ -429,4 +429,19 @@ mod tests { assert!(!opts.user_metadata.contains_key(X_AMZ_OBJECT_LOCK_LEGAL_HOLD.as_str())); assert!(!opts.user_metadata.contains_key(X_AMZ_REPLICATION_STATUS.as_str())); } + + #[test] + fn build_transition_put_options_requests_no_checksum_and_content_md5() { + // Regression for rustfs/rustfs#4811: transition uploads must leave the + // additional-checksum modes unset and rely on Content-MD5. If `checksum` + // were (incorrectly) reported as set, the >128 MiB multipart put path + // would call `ChecksumNone.hasher()` and fail with "unsupported checksum + // type". Objects <=128 MiB take the single-part path and only worked by + // silently dropping the checksum, so pin both invariants here. + let opts = build_transition_put_options("COLD".to_string(), HashMap::new()); + + assert!(!opts.checksum.is_set(), "transition put must not request an additional checksum"); + assert!(!opts.auto_checksum.is_set(), "transition put must not preset auto_checksum"); + assert!(opts.send_content_md5, "transition put must send Content-MD5"); + } }