From fc325bcc56076ee012c35e7923ae14028aa9b6d2 Mon Sep 17 00:00:00 2001 From: Justin Bradfield Date: Mon, 28 Sep 2026 05:51:48 -0500 Subject: [PATCH] fix(s3): report x-amz-mp-parts-count on GetObject with partNumber Merge the reviewed fix from pull request #8115. --- .../src/get_object_parts_count_test.rs | 153 ++++++++++++++++++ crates/e2e_test/src/lib.rs | 4 + rustfs/src/app/object/get.rs | 53 ++++++ 3 files changed, 210 insertions(+) create mode 100644 crates/e2e_test/src/get_object_parts_count_test.rs diff --git a/crates/e2e_test/src/get_object_parts_count_test.rs b/crates/e2e_test/src/get_object_parts_count_test.rs new file mode 100644 index 000000000..907d418e8 --- /dev/null +++ b/crates/e2e_test/src/get_object_parts_count_test.rs @@ -0,0 +1,153 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// 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. + +use crate::common::{RustFSTestEnvironment, init_logging}; +use aws_sdk_s3::primitives::ByteStream; +use aws_sdk_s3::types::{CompletedMultipartUpload, CompletedPart}; +use tracing::info; + +const PARTS_COUNT_BUCKET: &str = "parts-count-test-bucket"; +const MULTIPART_KEY: &str = "multipart-object.bin"; +const SINGLE_PUT_KEY: &str = "single-put-object.bin"; + +/// s3's minimum size for every multipart part but the last. +const PART_SIZE: usize = 5 * 1024 * 1024; +const LAST_PART_SIZE: usize = 1024; +const EXPECTED_PARTS: i32 = 3; + +#[tokio::test] +async fn get_object_reports_parts_count_for_multipart_upload() -> Result<(), Box> { + init_logging(); + info!("Starting GetObject x-amz-mp-parts-count regression test"); + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(Vec::new()).await?; + + let client = env.create_s3_client(); + env.create_test_bucket(PARTS_COUNT_BUCKET).await?; + + // A genuine multipart upload of three parts. + let created = client + .create_multipart_upload() + .bucket(PARTS_COUNT_BUCKET) + .key(MULTIPART_KEY) + .send() + .await?; + let upload_id = created.upload_id().expect("upload id"); + + let mut completed_parts = Vec::new(); + for part_number in 1..=EXPECTED_PARTS { + let size = if part_number == EXPECTED_PARTS { + LAST_PART_SIZE + } else { + PART_SIZE + }; + let uploaded = client + .upload_part() + .bucket(PARTS_COUNT_BUCKET) + .key(MULTIPART_KEY) + .upload_id(upload_id) + .part_number(part_number) + .body(ByteStream::from(vec![b'a' + (part_number as u8); size])) + .send() + .await?; + completed_parts.push( + CompletedPart::builder() + .part_number(part_number) + .e_tag(uploaded.e_tag().expect("part etag")) + .build(), + ); + } + + client + .complete_multipart_upload() + .bucket(PARTS_COUNT_BUCKET) + .key(MULTIPART_KEY) + .upload_id(upload_id) + .multipart_upload(CompletedMultipartUpload::builder().set_parts(Some(completed_parts)).build()) + .send() + .await?; + + let total_len = (PART_SIZE * 2 + LAST_PART_SIZE) as i64; + + // The header the fix is about. + let first_part = client + .get_object() + .bucket(PARTS_COUNT_BUCKET) + .key(MULTIPART_KEY) + .part_number(1) + .send() + .await?; + assert_eq!( + first_part.parts_count(), + Some(EXPECTED_PARTS), + "GetObject with partNumber must report the object's part count" + ); + assert_eq!( + first_part.content_length(), + Some(PART_SIZE as i64), + "the response body must be part 1, not the whole object" + ); + let expected_range = format!("bytes 0-{}/{}", PART_SIZE - 1, total_len); + assert_eq!( + first_part.content_range(), + Some(expected_range.as_str()), + "Content-Range must state part 1's extent and the whole object's size" + ); + + // Without partNumber s3 omits the header, so a plain GET is unchanged. + let whole = client + .get_object() + .bucket(PARTS_COUNT_BUCKET) + .key(MULTIPART_KEY) + .send() + .await?; + assert_eq!(whole.parts_count(), None, "a GET that does not name a part is owed no part count"); + assert_eq!(whole.content_length(), Some(total_len), "a plain GET returns the whole object"); + + // A single PutObject is not a multipart upload, so it is owed no count + // even when a part is named. + client + .put_object() + .bucket(PARTS_COUNT_BUCKET) + .key(SINGLE_PUT_KEY) + .body(ByteStream::from_static(b"0123456789abcdef")) + .send() + .await?; + let single = client + .get_object() + .bucket(PARTS_COUNT_BUCKET) + .key(SINGLE_PUT_KEY) + .part_number(1) + .send() + .await?; + assert_eq!(single.parts_count(), None, "an object stored by a single PutObject is not multipart"); + + client + .delete_object() + .bucket(PARTS_COUNT_BUCKET) + .key(MULTIPART_KEY) + .send() + .await?; + client + .delete_object() + .bucket(PARTS_COUNT_BUCKET) + .key(SINGLE_PUT_KEY) + .send() + .await?; + env.delete_test_bucket(PARTS_COUNT_BUCKET).await?; + env.stop_server(); + + Ok(()) +} diff --git a/crates/e2e_test/src/lib.rs b/crates/e2e_test/src/lib.rs index ea7fba936..b4e0f4ce9 100644 --- a/crates/e2e_test/src/lib.rs +++ b/crates/e2e_test/src/lib.rs @@ -232,6 +232,10 @@ mod tier_stats_cluster_test; #[cfg(test)] mod checksum_upload_test; +// GetObject with partNumber reporting x-amz-mp-parts-count +#[cfg(test)] +mod get_object_parts_count_test; + // Group deletion tests #[cfg(test)] mod group_delete_test; diff --git a/rustfs/src/app/object/get.rs b/rustfs/src/app/object/get.rs index 53f6837c4..aff55e805 100644 --- a/rustfs/src/app/object/get.rs +++ b/rustfs/src/app/object/get.rs @@ -2276,6 +2276,17 @@ impl DefaultObjectUsecase { Self::validate_get_object_part_number(part_number, info) } + /// The `x-amz-mp-parts-count` value for a response, if one is owed. + /// + /// s3 reports the part count only on a response to a request that named a + /// part, and only for an object created by a multipart upload. + fn get_object_parts_count(part_number: Option, info: &ObjectInfo) -> Option { + if part_number.is_none() || !info.is_multipart() { + return None; + } + i32::try_from(info.parts.len()).ok() + } + /// How long a GET waits for a disk read permit before degrading to a /// permit-less read. Cached: consulted per GET. Zero disables the bound. fn disk_permit_wait_timeout() -> Duration { @@ -3720,6 +3731,8 @@ impl DefaultObjectUsecase { let metadata = filter_object_metadata(&info.user_defined); record_get_object_s3_handler_stage_duration(GET_OBJECT_STAGE_METADATA_FILTER, metadata_filter_start); + let parts_count = Self::get_object_parts_count(part_number, &info); + let output = GetObjectOutput { body: Some(body), content_length: Some(response_content_length), @@ -3730,6 +3743,7 @@ impl DefaultObjectUsecase { cache_control, content_disposition, content_range, + parts_count, e_tag: info.etag.map(|etag| to_s3s_etag(&etag)), metadata, website_redirect_location: req @@ -10824,6 +10838,45 @@ mod tests { assert!(DefaultObjectUsecase::validate_get_object_part_number(Some(1), &info).is_ok()); } + #[test] + fn get_object_reports_parts_count_for_multipart_objects() { + fn info_with_parts(count: usize, etag: &str) -> ObjectInfo { + ObjectInfo { + parts: Arc::new( + (1..=count) + .map(|number| rustfs_filemeta::ObjectPartInfo { + number, + ..Default::default() + }) + .collect(), + ), + etag: Some(etag.to_string()), + ..Default::default() + } + } + + // A multipart object, asked for by part: the count is what lets the + // client know three more parts follow. + let multipart = info_with_parts(4, "d41d8cd98f00b204e9800998ecf8427e-4"); + assert!(multipart.is_multipart()); + assert_eq!(DefaultObjectUsecase::get_object_parts_count(Some(1), &multipart), Some(4)); + + // Not asked for by part: s3 omits the header entirely. + assert_eq!(DefaultObjectUsecase::get_object_parts_count(None, &multipart), None); + + // A plain PutObject is stored as one part and carries a bare 32-char + // etag, so it is not multipart and is owed no count. + let single = info_with_parts(1, "d41d8cd98f00b204e9800998ecf8427e"); + assert!(!single.is_multipart()); + assert_eq!(DefaultObjectUsecase::get_object_parts_count(Some(1), &single), None); + + // A multipart upload that happened to have one part still carries the + // `-1` etag suffix, so it is multipart and reports a count of 1. + let single_part_multipart = info_with_parts(1, "d41d8cd98f00b204e9800998ecf8427e-1"); + assert!(single_part_multipart.is_multipart()); + assert_eq!(DefaultObjectUsecase::get_object_parts_count(Some(1), &single_part_multipart), Some(1)); + } + #[test] fn cold_fill_conditions_fail_before_phase_probe_advances() { fn run_phase_probe(headers: &HeaderMap, info: &ObjectInfo) -> (S3Result<()>, [usize; 3]) {