fix(s3): report x-amz-mp-parts-count on GetObject with partNumber

Merge the reviewed fix from pull request #8115.
This commit is contained in:
Justin Bradfield
2026-09-28 05:51:48 -05:00
committed by GitHub
parent 4a46f5ff3d
commit fc325bcc56
3 changed files with 210 additions and 0 deletions
@@ -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<dyn std::error::Error + Send + Sync>> {
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(())
}
+4
View File
@@ -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;
+53
View File
@@ -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<usize>, info: &ObjectInfo) -> Option<i32> {
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]) {