From d91dfcedd25f8b1418fddc1cf5287fde09cbbabe Mon Sep 17 00:00:00 2001 From: Chris Date: Sat, 24 Oct 2026 09:17:48 +0900 Subject: [PATCH] refactor(s3): migrate private multipart validation errors (#8844) --- .config/s3s-footprint-baseline.txt | 2 +- rustfs/src/app/multipart_usecase.rs | 45 +++++++++++++++-------------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/.config/s3s-footprint-baseline.txt b/.config/s3s-footprint-baseline.txt index 9de22b106..eb29293df 100644 --- a/.config/s3s-footprint-baseline.txt +++ b/.config/s3s-footprint-baseline.txt @@ -13,5 +13,5 @@ # header's rule clears (rustfs_s3_types's macro); fails closed # ecstore_files Rust files referencing s3s paths under crates/ecstore/src files=107 -s3_error_lines=294 +s3_error_lines=290 ecstore_files=0 diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index b65921624..9ee83bdf5 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -174,25 +174,25 @@ fn validate_copy_source_range_not_exceeds(range_spec: &HTTPRangeSpec, object_siz Ok(()) } -fn validate_complete_multipart_parts(parts: &[CompletePart]) -> S3Result<()> { +fn validate_complete_multipart_parts(parts: &[CompletePart]) -> rustfs_s3_types::S3Result<()> { if let Some(part) = parts .iter() .find(|part| !(1..=MAX_MULTIPART_PART_NUMBER as usize).contains(&part.part_num)) { - return Err(S3Error::with_message( - S3ErrorCode::InvalidPart, + return Err(rustfs_s3_types::S3Error::with_message( + rustfs_s3_types::S3ErrorCode::InvalidPart, format!("Part number {} must be between 1 and {MAX_MULTIPART_PART_NUMBER}", part.part_num), )); } if parts.windows(2).any(|window| window[0].part_num >= window[1].part_num) { - return Err(s3_error!(InvalidPartOrder, "Part numbers must be strictly increasing")); + return Err(rustfs_s3_types::s3_error!(InvalidPartOrder, "Part numbers must be strictly increasing")); } Ok(()) } -fn normalize_complete_multipart_parts(parts: Vec) -> S3Result> { +fn normalize_complete_multipart_parts(parts: Vec) -> rustfs_s3_types::S3Result> { // For duplicate part numbers, keep the last occurrence from the request. // This matches retry/resend semantics where later uploads override earlier ones. let mut seen = HashSet::with_capacity(parts.len()); @@ -383,22 +383,22 @@ fn extract_request_host(headers: &HeaderMap, uri: &Uri) -> Option { .or_else(|| uri.authority().map(|authority| authority.as_str().to_string())) } -fn resolve_upload_part_size(content_length: Option, body: Option<&StreamingBlob>) -> S3Result> { +fn resolve_upload_part_size(content_length: Option, body: Option<&StreamingBlob>) -> rustfs_s3_types::S3Result> { if let Some(length) = content_length { return Ok(Some(length)); } body.and_then(|body| body.remaining_length().exact()) .map(i64::try_from) .transpose() - .map_err(|_| s3_error!(UnexpectedContent)) + .map_err(|_| rustfs_s3_types::s3_error!(UnexpectedContent)) } -fn require_upload_part_size(size: Option, capped: bool) -> S3Result { +fn require_upload_part_size(size: Option, capped: bool) -> rustfs_s3_types::S3Result { match size { Some(size) if size >= 0 => Ok(size), - Some(_) => Err(s3_error!(UnexpectedContent)), - None if capped => Err(s3_error!(UnexpectedContent)), - None => Err(S3Error::new(S3ErrorCode::MissingContentLength)), + Some(_) => Err(rustfs_s3_types::s3_error!(UnexpectedContent)), + None if capped => Err(rustfs_s3_types::s3_error!(UnexpectedContent)), + None => Err(rustfs_s3_types::S3Error::new(rustfs_s3_types::S3ErrorCode::MissingContentLength)), } } @@ -2374,7 +2374,8 @@ mod tests { #[test] fn resolve_upload_part_size_uses_normalized_logical_length() { - assert_eq!(resolve_upload_part_size(Some(5242880), None).expect("DTO length"), Some(5242880)); + let result: rustfs_s3_types::S3Result> = resolve_upload_part_size(Some(5242880), None); + assert_eq!(result.expect("DTO length"), Some(5242880)); assert_eq!(resolve_upload_part_size(None, None).expect("unknown length"), None); let body = StreamingBlob::from(Bytes::from_static(b"abc")); assert_eq!(resolve_upload_part_size(None, Some(&body)).expect("exact bytes"), Some(3)); @@ -2386,19 +2387,20 @@ mod tests { #[test] fn upload_part_length_contract_rejects_unknown_and_all_negative_lengths() { + let result: rustfs_s3_types::S3Result = require_upload_part_size(None, false); assert_eq!( - require_upload_part_size(None, false).expect_err("ordinary unknown").code(), - &S3ErrorCode::MissingContentLength + result.expect_err("ordinary unknown").code(), + &rustfs_s3_types::S3ErrorCode::MissingContentLength ); assert_eq!( require_upload_part_size(None, true).expect_err("capped unknown").code(), - &S3ErrorCode::UnexpectedContent + &rustfs_s3_types::S3ErrorCode::UnexpectedContent ); for capped in [false, true] { for size in [-1, -2, i64::MIN] { assert_eq!( require_upload_part_size(Some(size), capped).expect_err("negative").code(), - &S3ErrorCode::UnexpectedContent + &rustfs_s3_types::S3ErrorCode::UnexpectedContent ); } assert_eq!(require_upload_part_size(Some(0), capped).expect("zero is valid"), 0); @@ -3213,11 +3215,11 @@ mod tests { #[test] fn validate_complete_multipart_parts_enforces_part_number_range() { - validate_complete_multipart_parts(&[CompletePart { + let result: rustfs_s3_types::S3Result<()> = validate_complete_multipart_parts(&[CompletePart { part_num: MAX_MULTIPART_PART_NUMBER as usize, ..Default::default() - }]) - .expect("part number 10000 must remain valid"); + }]); + result.expect("part number 10000 must remain valid"); for part_num in [0, MAX_MULTIPART_PART_NUMBER as usize + 1] { let err = validate_complete_multipart_parts(&[CompletePart { @@ -3225,7 +3227,7 @@ mod tests { ..Default::default() }]) .expect_err("out-of-range complete part number must be rejected"); - assert_eq!(err.code(), &S3ErrorCode::InvalidPart); + assert_eq!(err.code(), &rustfs_s3_types::S3ErrorCode::InvalidPart); let expected = format!("Part number {part_num} must be between 1 and {MAX_MULTIPART_PART_NUMBER}"); assert_eq!(err.message(), Some(expected.as_str())); } @@ -3246,7 +3248,8 @@ mod tests { }, ]; - let normalized = normalize_complete_multipart_parts(input).expect("normalization should succeed"); + let result: rustfs_s3_types::S3Result> = normalize_complete_multipart_parts(input); + let normalized = result.expect("normalization should succeed"); assert_eq!(normalized.len(), 1); assert_eq!(normalized[0].part_num, 1); assert_eq!(normalized[0].etag.as_deref(), Some("new"));