diff --git a/crates/s3-client/src/api_get_object.rs b/crates/s3-client/src/api_get_object.rs index 9f6f49d59..cc06fe388 100644 --- a/crates/s3-client/src/api_get_object.rs +++ b/crates/s3-client/src/api_get_object.rs @@ -69,7 +69,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), diff --git a/crates/s3-client/src/api_list.rs b/crates/s3-client/src/api_list.rs index 3bd1f1edd..c2771d081 100644 --- a/crates/s3-client/src/api_list.rs +++ b/crates/s3-client/src/api_list.rs @@ -103,7 +103,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -206,7 +205,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), diff --git a/crates/s3-client/src/api_put_object_multipart.rs b/crates/s3-client/src/api_put_object_multipart.rs index 17efdbf3a..d7d1ba7a1 100644 --- a/crates/s3-client/src/api_put_object_multipart.rs +++ b/crates/s3-client/src/api_put_object_multipart.rs @@ -26,7 +26,7 @@ use time::OffsetDateTime; use tracing::warn; use uuid::Uuid; -use crate::checksum::ChecksumMode; +use crate::checksum::{ChecksumMode, checksum_header_value}; use crate::utils::base64_encode; use crate::{ api_error_response::{ @@ -223,7 +223,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -306,7 +305,6 @@ impl TransitionClient { stream_sha256: p.stream_sha256, trailer: p.trailer.clone(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -329,31 +327,11 @@ impl TransitionClient { //} let h = resp.headers(); let mut obj_part = ObjectPart { - checksum_crc32: if let Some(h_checksum_crc32) = h.get(ChecksumMode::ChecksumCRC32.key()) { - h_checksum_crc32.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_crc32c: if let Some(h_checksum_crc32c) = h.get(ChecksumMode::ChecksumCRC32C.key()) { - h_checksum_crc32c.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_sha1: if let Some(h_checksum_sha1) = h.get(ChecksumMode::ChecksumSHA1.key()) { - h_checksum_sha1.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_sha256: if let Some(h_checksum_sha256) = h.get(ChecksumMode::ChecksumSHA256.key()) { - h_checksum_sha256.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_crc64nvme: if let Some(h_checksum_crc64nvme) = h.get(ChecksumMode::ChecksumCRC64NVME.key()) { - h_checksum_crc64nvme.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, + checksum_crc32: checksum_header_value(h, ChecksumMode::ChecksumCRC32), + checksum_crc32c: checksum_header_value(h, ChecksumMode::ChecksumCRC32C), + checksum_sha1: checksum_header_value(h, ChecksumMode::ChecksumSHA1), + checksum_sha256: checksum_header_value(h, ChecksumMode::ChecksumSHA256), + checksum_crc64nvme: checksum_header_value(h, ChecksumMode::ChecksumCRC64NVME), ..Default::default() }; obj_part.size = p.size; @@ -393,7 +371,6 @@ impl TransitionClient { stream_sha256: Default::default(), trailer: Default::default(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), diff --git a/crates/s3-client/src/api_put_object_streaming.rs b/crates/s3-client/src/api_put_object_streaming.rs index 9ae54b893..8c9c7b5a1 100644 --- a/crates/s3-client/src/api_put_object_streaming.rs +++ b/crates/s3-client/src/api_put_object_streaming.rs @@ -31,7 +31,7 @@ use tokio_util::sync::CancellationToken; use tracing::warn; use uuid::Uuid; -use crate::checksum::{ChecksumMode, add_auto_checksum_headers, apply_auto_checksum}; +use crate::checksum::{ChecksumMode, add_auto_checksum_headers, apply_auto_checksum, checksum_header_value}; use crate::{ api_error_response::{err_invalid_argument, err_unexpected_eof, http_resp_to_error_response}, api_put_object::PutObjectOptions, @@ -503,7 +503,6 @@ impl TransitionClient { content_md5_base64: md5_base64.to_string(), content_sha256_hex: sha256_hex.to_string(), stream_sha256: !opts.disable_content_sha256, - add_crc: Default::default(), bucket_location: Default::default(), pre_sign_url: Default::default(), query_values: Default::default(), @@ -511,21 +510,7 @@ impl TransitionClient { expires: Default::default(), trailer: Default::default(), }; - let mut add_crc = false; //self.trailing_header_support && md5_base64 == "" && !s3utils.IsGoogleEndpoint(self.endpoint_url) && (opts.disable_content_sha256 || self.secure); - let mut opts = opts.clone(); - if opts.checksum.is_set() { - req_metadata.add_crc = opts.checksum; - } else if add_crc { - for (k, _) in opts.user_metadata { - if k.to_lowercase().starts_with("x-amz-checksum-") { - add_crc = false; - } - } - if add_crc { - opts.auto_checksum.set_default(ChecksumMode::ChecksumCRC32C); - req_metadata.add_crc = opts.auto_checksum; - } - } + let opts = opts.clone(); if opts.internal.source_version_id != "" { if !opts.internal.source_version_id.is_empty() { @@ -570,31 +555,11 @@ impl TransitionClient { size, expiration: exp_time, expiration_rule_id: rule_id, - checksum_crc32: if let Some(h_checksum_crc32) = h.get(ChecksumMode::ChecksumCRC32.key()) { - h_checksum_crc32.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_crc32c: if let Some(h_checksum_crc32c) = h.get(ChecksumMode::ChecksumCRC32C.key()) { - h_checksum_crc32c.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_sha1: if let Some(h_checksum_sha1) = h.get(ChecksumMode::ChecksumSHA1.key()) { - h_checksum_sha1.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_sha256: if let Some(h_checksum_sha256) = h.get(ChecksumMode::ChecksumSHA256.key()) { - h_checksum_sha256.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, - checksum_crc64nvme: if let Some(h_checksum_crc64nvme) = h.get(ChecksumMode::ChecksumCRC64NVME.key()) { - h_checksum_crc64nvme.to_str().unwrap_or("").to_string() - } else { - "".to_string() - }, + checksum_crc32: checksum_header_value(h, ChecksumMode::ChecksumCRC32), + checksum_crc32c: checksum_header_value(h, ChecksumMode::ChecksumCRC32C), + checksum_sha1: checksum_header_value(h, ChecksumMode::ChecksumSHA1), + checksum_sha256: checksum_header_value(h, ChecksumMode::ChecksumSHA256), + checksum_crc64nvme: checksum_header_value(h, ChecksumMode::ChecksumCRC64NVME), ..Default::default() }) } diff --git a/crates/s3-client/src/api_remove.rs b/crates/s3-client/src/api_remove.rs index 3002e71ca..5e063e14f 100644 --- a/crates/s3-client/src/api_remove.rs +++ b/crates/s3-client/src/api_remove.rs @@ -99,7 +99,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -131,7 +130,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -185,7 +183,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -347,7 +344,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -407,7 +403,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), diff --git a/crates/s3-client/src/api_s3_datatypes.rs b/crates/s3-client/src/api_s3_datatypes.rs index 2f6475815..d17eb6cbb 100644 --- a/crates/s3-client/src/api_s3_datatypes.rs +++ b/crates/s3-client/src/api_s3_datatypes.rs @@ -262,32 +262,6 @@ pub struct CompletePart { pub checksum_crc64nvme: String, } -impl CompletePart { - #[allow(dead_code, reason = "MinIO-parity accessor with no caller in this port (backlog#1823)")] - fn checksum(&self, t: &ChecksumMode) -> String { - match t { - ChecksumMode::ChecksumCRC32C => { - return self.checksum_crc32c.clone(); - } - ChecksumMode::ChecksumCRC32 => { - return self.checksum_crc32.clone(); - } - ChecksumMode::ChecksumSHA1 => { - return self.checksum_sha1.clone(); - } - ChecksumMode::ChecksumSHA256 => { - return self.checksum_sha256.clone(); - } - ChecksumMode::ChecksumCRC64NVME => { - return self.checksum_crc64nvme.clone(); - } - _ => { - return "".to_string(); - } - } - } -} - #[derive(Debug, Default, serde::Serialize)] #[serde(rename = "CompleteMultipartUpload")] pub struct CompleteMultipartUpload { diff --git a/crates/s3-client/src/api_stat.rs b/crates/s3-client/src/api_stat.rs index c4bc8c9d8..bac59c8ed 100644 --- a/crates/s3-client/src/api_stat.rs +++ b/crates/s3-client/src/api_stat.rs @@ -104,7 +104,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -159,7 +158,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), @@ -212,7 +210,6 @@ impl TransitionClient { stream_sha256: false, trailer: HeaderMap::new(), pre_sign_url: Default::default(), - add_crc: Default::default(), extra_pre_sign_header: Default::default(), bucket_location: Default::default(), expires: Default::default(), diff --git a/crates/s3-client/src/checksum.rs b/crates/s3-client/src/checksum.rs index b85abb06b..06aacdb06 100644 --- a/crates/s3-client/src/checksum.rs +++ b/crates/s3-client/src/checksum.rs @@ -1,4 +1,3 @@ -#![allow(clippy::map_entry)] // Copyright 2024 RustFS Team // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -12,17 +11,10 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -#![allow(unused_imports)] -#![allow(unused_variables)] -#![allow(unused_mut)] -#![allow(unused_assignments)] -#![allow(unused_must_use)] -#![allow(clippy::all)] 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}; @@ -104,10 +96,6 @@ impl ChecksumMode { .is_some_and(|a| a.supports_full_object() && !a.supports_composite()) } - pub fn key_capitalized(&self) -> String { - self.key() - } - pub fn raw_byte_len(&self) -> usize { self.algorithm().map(|a| a.raw_len()).unwrap_or(0) } @@ -144,39 +132,13 @@ impl ChecksumMode { Ok(base64_encode(hash.as_ref())) } - pub fn to_string(&self) -> String { - match self.algorithm() { - Some(algorithm) => algorithm.s3_algorithm_name().to_string(), - None if matches!(self, ChecksumMode::ChecksumNone) => "".to_string(), - None => "".to_string(), - } - } - - // pub fn check_sum_reader(&self, r: GetObjectReader) -> Result { - // let mut h = self.hasher()?; - // Ok(Checksum::new(self.clone(), h.sum().as_bytes())) - // } - - // pub fn check_sum_bytes(&self, b: &[u8]) -> Result { - // let mut h = self.hasher()?; - // Ok(Checksum::new(self.clone(), h.sum().as_bytes())) - // } - pub fn composite_checksum(&self, p: &mut [ObjectPart]) -> Result { if !self.can_composite() { return Err(std::io::Error::other("cannot do composite checksum")); } - p.sort_by(|i, j| { - if i.part_num < j.part_num { - std::cmp::Ordering::Less - } else if i.part_num > j.part_num { - std::cmp::Ordering::Greater - } else { - std::cmp::Ordering::Equal - } - }); + p.sort_by_key(|part| part.part_num); let c = self.base(); - let mut crc_bytes = Vec::::with_capacity(p.len() * self.raw_byte_len() as usize); + let mut crc_bytes = Vec::::with_capacity(p.len() * self.raw_byte_len()); let mut h = self.hasher()?; for part in p.iter() { let part_checksum = part.checksum_raw(&c)?; @@ -185,9 +147,8 @@ impl ChecksumMode { h.update(crc_bytes.as_ref()); let hash = h.finalize(); Ok(Checksum { - checksum_type: self.clone(), + checksum_type: *self, r: hash.as_ref().to_vec(), - computed: false, }) } @@ -200,6 +161,19 @@ impl ChecksumMode { } } +impl std::fmt::Display for ChecksumMode { + /// The `x-amz-checksum-algorithm` wire value: the algorithm name for + /// concrete modes, `""` for `ChecksumNone`, `""` for a bare + /// `ChecksumFullObject` flag. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.algorithm() { + Some(algorithm) => f.write_str(algorithm.s3_algorithm_name()), + None if matches!(self, ChecksumMode::ChecksumNone) => Ok(()), + None => f.write_str(""), + } + } +} + #[cfg(test)] mod tests { use super::ChecksumMode; @@ -332,7 +306,6 @@ mod tests { 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); @@ -351,6 +324,24 @@ mod tests { } } + #[test] + fn test_checksum_header_value_reads_present_absent_and_invalid() { + use super::checksum_header_value; + use http::{HeaderMap, HeaderValue}; + + let mut headers = HeaderMap::new(); + headers.insert("x-amz-checksum-crc32c", HeaderValue::from_static("yZRlqg==")); + headers.insert("x-amz-checksum-sha256", HeaderValue::from_bytes(b"\xff\xfe").unwrap()); + + // Present header returns its value verbatim. + assert_eq!(checksum_header_value(&headers, ChecksumMode::ChecksumCRC32C), "yZRlqg=="); + // Absent header and the unset modes (whose key() is "") return "". + assert_eq!(checksum_header_value(&headers, ChecksumMode::ChecksumCRC32), ""); + assert_eq!(checksum_header_value(&headers, ChecksumMode::ChecksumNone), ""); + // A non-UTF-8 header value degrades to "" instead of erroring. + assert_eq!(checksum_header_value(&headers, ChecksumMode::ChecksumSHA256), ""); + } + #[test] fn test_set_default_upgrades_none() { // With `is_set()` fixed, `set_default` must upgrade an unset mode to the @@ -371,42 +362,9 @@ mod tests { pub struct Checksum { checksum_type: ChecksumMode, r: Vec, - #[allow( - dead_code, - reason = "checksum bookkeeping field kept beside the value it guards (backlog#1823)" - )] - computed: bool, } impl Checksum { - #[allow(dead_code, reason = "MinIO-parity surface with no caller in this port (backlog#1823)")] - fn new(t: ChecksumMode, b: &[u8]) -> Checksum { - if t.is_set() && b.len() == t.raw_byte_len() { - return Checksum { - checksum_type: t, - r: b.to_vec(), - computed: false, - }; - } - Checksum::default() - } - - #[allow(dead_code, reason = "MinIO-parity surface with no caller in this port (backlog#1823)")] - fn new_checksum_string(t: ChecksumMode, s: &str) -> Result { - let b = match base64_decode(s.as_bytes()) { - Ok(b) => b, - Err(err) => return Err(std::io::Error::other(err.to_string())), - }; - if t.is_set() && b.len() == t.raw_byte_len() { - return Ok(Checksum { - checksum_type: t, - r: b, - computed: false, - }); - } - Ok(Checksum::default()) - } - fn is_set(&self) -> bool { self.checksum_type.is_set() && self.r.len() == self.checksum_type.raw_byte_len() } @@ -417,14 +375,17 @@ impl Checksum { } base64_encode(&self.r) } +} - #[allow(dead_code, reason = "MinIO-parity surface with no caller in this port (backlog#1823)")] - fn raw(&self) -> Option> { - if !self.is_set() { - return None; - } - Some(self.r.clone()) - } +/// Read the base64 digest carried by `mode`'s `x-amz-checksum-*` response +/// header, or an empty string when the header is absent (the client's +/// datatypes use `""` for "no checksum"). +pub fn checksum_header_value(headers: &http::HeaderMap, mode: ChecksumMode) -> String { + headers + .get(mode.key()) + .and_then(|value| value.to_str().ok()) + .unwrap_or("") + .to_string() } pub fn add_auto_checksum_headers(opts: &mut PutObjectOptions) { @@ -448,7 +409,7 @@ pub fn apply_auto_checksum(opts: &mut PutObjectOptions, all_parts: &mut [ObjectP let crc = opts.auto_checksum.full_object_checksum(all_parts)?; opts.user_metadata = { let mut hm = HashMap::new(); - hm.insert(opts.auto_checksum.key_capitalized(), crc.encoded()); + hm.insert(opts.auto_checksum.key(), crc.encoded()); hm.insert("X-Amz-Checksum-Type".to_string(), "FULL_OBJECT".to_string()); hm } diff --git a/crates/s3-client/src/transition_api.rs b/crates/s3-client/src/transition_api.rs index 87614ee84..6f45c76e2 100644 --- a/crates/s3-client/src/transition_api.rs +++ b/crates/s3-client/src/transition_api.rs @@ -19,7 +19,6 @@ #![allow(clippy::all)] use crate::bucket_cache::BucketLocationCache; -use crate::checksum::ChecksumMode; use crate::{ api_error_response::ErrorResponse, api_error_response::{err_invalid_argument, http_resp_to_error_response, to_error_response}, @@ -898,7 +897,6 @@ pub struct RequestMetadata { pub content_md5_base64: String, pub content_sha256_hex: String, pub stream_sha256: bool, - pub add_crc: ChecksumMode, pub trailer: HeaderMap, } diff --git a/scripts/ecstore-module-lint-register.txt b/scripts/ecstore-module-lint-register.txt index ccb10b437..bf777b0f7 100644 --- a/scripts/ecstore-module-lint-register.txt +++ b/scripts/ecstore-module-lint-register.txt @@ -59,9 +59,6 @@ crates/s3-client/src/api_stat.rs|unused_variables crates/s3-client/src/bucket_cache.rs|clippy::all crates/s3-client/src/bucket_cache.rs|unused_must_use crates/s3-client/src/bucket_cache.rs|unused_variables -crates/s3-client/src/checksum.rs|clippy::all -crates/s3-client/src/checksum.rs|unused_must_use -crates/s3-client/src/checksum.rs|unused_variables crates/s3-client/src/constants.rs|unused_must_use crates/s3-client/src/constants.rs|unused_variables crates/s3-client/src/credentials.rs|clippy::all