refactor(ecstore): make transition-client base64 standard everywhere (#4857)

refactor(ecstore): make client base64 standard everywhere, drop the split

Self-review of the transition-client fixes: the Content-MD5 fix had only
converted the two transition call sites to a parallel base64_encode_standard,
leaving the same URL-safe, unpadded base64 bug on every other outbound value —
the SigV2/no-length multipart Content-MD5, the x-amz-checksum-* headers on the
parallel and no-length paths, api_remove's multi-delete Content-MD5, and the
checksum encode/decode helpers. Every base64 value this client emits or parses
is S3 wire format, which is standard base64; none is ever URL-safe.

Encode and decode both use base64_simd::STANDARD now, and the parallel
base64_encode_standard is gone. This fixes those seven latent call sites at the
root and removes the duplicate function. The transition path is functionally
unchanged (it already produced standard base64 via the helper), so the
end-to-end byte-for-byte result is preserved.

Also drop a stray Vec::with_capacity(part_size) that was allocated (up to
128 MiB) and immediately overwritten by read_multipart_part's own buffer.

Refs: rustfs/rustfs#4811, rustfs/backlog#1267

Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
houseme
2026-07-15 15:56:50 +08:00
committed by GitHub
parent d3669ed637
commit 4f3f2afd0d
2 changed files with 23 additions and 28 deletions
@@ -42,7 +42,7 @@ use crate::client::{
transition_api::{ReaderImpl, RequestMetadata, TransitionClient, UploadInfo},
};
use crate::client::utils::{base64_encode, base64_encode_standard};
use crate::client::utils::base64_encode;
use rustfs_utils::path::trim_etag;
use s3s::header::{X_AMZ_EXPIRATION, X_AMZ_VERSION_ID};
@@ -161,7 +161,6 @@ impl TransitionClient {
let mut total_uploaded_size: i64 = 0;
let mut parts_info = HashMap::<i64, ObjectPart>::new();
let mut buf = Vec::<u8>::with_capacity(part_size as usize);
let mut md5_base64: String = "".to_string();
for part_number in 1..=total_parts_count {
@@ -174,7 +173,7 @@ impl TransitionClient {
// 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 buf = read_multipart_part(&mut reader, part_size as usize).await?;
let length = buf.len();
if opts.send_content_md5 {
@@ -184,16 +183,14 @@ impl TransitionClient {
None => return Err(std::io::Error::other("MD5 hasher not initialized")),
};
let hash = md5_hash.hash_encode(&buf[..length]);
// Content-MD5 must be standard base64 for the remote to accept it.
md5_base64 = base64_encode_standard(hash.as_ref());
md5_base64 = base64_encode(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()) {
// x-amz-checksum-* header values are standard base64 too.
if let Ok(header_value) = base64_encode_standard(csum.as_ref()).parse() {
if let Ok(header_value) = base64_encode(csum.as_ref()).parse() {
custom_header.insert(header_name, header_value);
} else {
warn!("Failed to parse checksum value");
+19 -21
View File
@@ -91,43 +91,41 @@ pub fn is_minio_header(header_key: &str) -> bool {
header_key.to_lowercase().starts_with("x-minio-")
}
/// Standard base64 (with `+`/`/` and `=` padding). Every base64 value this
/// transition client emits or parses — `Content-MD5`, `x-amz-checksum-*`, and
/// checksum digests in request/response bodies — is S3 wire format, which is
/// standard base64. The URL-safe, unpadded alphabet used previously made remotes
/// reject `Content-MD5` with "Invalid content MD5: Base64Error" and could not
/// even decode a padded checksum coming back from the peer (rustfs/rustfs#4811).
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<Vec<u8>, base64_simd::Error> {
base64_simd::URL_SAFE_NO_PAD.decode_to_vec(input)
base64_simd::STANDARD.decode_to_vec(input)
}
#[cfg(test)]
mod tests {
use super::base64_encode_standard;
use super::{base64_decode, base64_encode};
#[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.
fn base64_encode_is_standard_s3_wire_format() {
// S3 reads Content-MD5 / checksum values with a standard base64 decoder,
// so the encoder must emit '+'/'/' and '=' padding and round-trip through
// one. Regression for rustfs/rustfs#4811 ("Invalid content MD5:
// Base64Error"). 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);
let encoded = base64_encode(&digest);
assert!(encoded.ends_with('='), "16-byte input must be padded: {encoded}");
let decoded = base64_simd::STANDARD
assert!(!encoded.contains(['-', '_']), "must use the standard alphabet: {encoded}");
let via_standard = base64_simd::STANDARD
.decode_to_vec(encoded.as_bytes())
.expect("standard decode");
assert_eq!(decoded, digest);
assert_eq!(via_standard, digest);
// Our own decoder must accept the same wire format it produces.
assert_eq!(base64_decode(encoded.as_bytes()).expect("round-trip"), digest);
}
}