From fd2a87d47ecc3ed4e6f04e637c58426781626489 Mon Sep 17 00:00:00 2001 From: cxymds Date: Tue, 28 Jul 2026 16:35:50 +0800 Subject: [PATCH] fix(s3): reject empty multipart completions (#5352) Co-authored-by: houseme --- rustfs/src/app/multipart_usecase.rs | 134 ++++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 6 deletions(-) diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index 5d4e208d1..a926be8f3 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -440,6 +440,12 @@ impl DefaultMultipartUsecase { } let Some(multipart_upload) = multipart_upload else { return Err(s3_error!(InvalidPart)) }; + let Some(parts) = multipart_upload.parts.filter(|parts| !parts.is_empty()) else { + return Err(S3Error::with_message( + S3ErrorCode::InvalidRequest, + "You must specify at least one part".to_string(), + )); + }; let mut opts = get_complete_multipart_upload_opts(&req.headers).map_err(ApiError::from)?; let versioned = BucketVersioningSys::prefix_enabled(&bucket, &key).await; @@ -449,12 +455,7 @@ impl DefaultMultipartUsecase { let capacity_scope_token = Uuid::new_v4(); opts.capacity_scope_token = Some(capacity_scope_token); - let uploaded_parts_vec = multipart_upload - .parts - .unwrap_or_default() - .into_iter() - .map(complete_part_from_s3) - .collect::>(); + let uploaded_parts_vec = parts.into_iter().map(complete_part_from_s3).collect::>(); let uploaded_parts = normalize_complete_multipart_parts(uploaded_parts_vec)?; @@ -1793,6 +1794,127 @@ mod tests { assert_eq!(err.code(), &S3ErrorCode::InvalidPart); } + #[tokio::test] + async fn execute_complete_multipart_upload_rejects_missing_parts_list() { + use s3s::xml::Deserialize as _; + + let mut deserializer = s3s::xml::Deserializer::new(b""); + let multipart_upload = + CompletedMultipartUpload::deserialize(&mut deserializer).expect("empty multipart XML should decode"); + deserializer + .expect_eof() + .expect("empty multipart XML should be fully consumed"); + assert!(multipart_upload.parts.is_none()); + + let input = CompleteMultipartUploadInput::builder() + .bucket("bucket".to_string()) + .key("object".to_string()) + .upload_id("upload-id".to_string()) + .multipart_upload(Some(multipart_upload)) + .build() + .expect("complete multipart input should build"); + let req = build_request(input, Method::POST); + + let err = Box::pin(make_usecase().execute_complete_multipart_upload(req)) + .await + .expect_err("missing parts list must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + } + + #[tokio::test] + #[serial_test::serial] + async fn rejected_empty_parts_preserve_existing_object_and_staging() { + use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions}; + + let store = crate::app::gating_test_env::shared_gating_ecstore().await; + crate::app::runtime_sources::install_test_app_context(Arc::clone(&store)).await; + + let bucket = format!("empty-complete-{}", Uuid::new_v4()); + let object = "existing-object"; + let existing_payload = b"existing object must survive"; + store + .make_bucket(&bucket, &MakeBucketOptions::default()) + .await + .expect("create multipart regression bucket"); + + let mut existing_reader = PutObjReader::from_vec(existing_payload.to_vec()); + let existing_info = store + .put_object(&bucket, object, &mut existing_reader, &ObjectOptions::default()) + .await + .expect("write existing object"); + + let upload = store + .new_multipart_upload(&bucket, object, &ObjectOptions::default()) + .await + .expect("create multipart staging upload"); + let mut part_reader = PutObjReader::from_vec(b"staged part".to_vec()); + let staged_part = store + .put_object_part(&bucket, object, &upload.upload_id, 1, &mut part_reader, &ObjectOptions::default()) + .await + .expect("write staged multipart part"); + + let input = CompleteMultipartUploadInput::builder() + .bucket(bucket.clone()) + .key(object.to_string()) + .upload_id(upload.upload_id.clone()) + .multipart_upload(Some(CompletedMultipartUpload { parts: Some(Vec::new()) })) + .build() + .expect("complete multipart input should build"); + let err = DefaultMultipartUsecase::from_global() + .execute_complete_multipart_upload(build_request(input, Method::POST)) + .await + .expect_err("empty parts list must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + + let current_info = store + .get_object_info(&bucket, object, &ObjectOptions::default()) + .await + .expect("existing object should remain readable"); + assert_eq!(current_info.size, existing_info.size); + assert_eq!(current_info.etag, existing_info.etag); + + let staged = store + .list_object_parts(&bucket, object, &upload.upload_id, None, 1000, &ObjectOptions::default()) + .await + .expect("multipart staging should remain available"); + assert_eq!(staged.parts.len(), 1); + assert_eq!(staged.parts[0].part_num, 1); + assert_eq!(staged.parts[0].etag, staged_part.etag); + + let staged_etag = staged_part.etag.expect("staged part should have an ETag"); + let input = CompleteMultipartUploadInput::builder() + .bucket(bucket) + .key(object.to_string()) + .upload_id(upload.upload_id) + .multipart_upload(Some(CompletedMultipartUpload { + parts: Some(vec![CompletedPart { + part_number: Some(1), + e_tag: Some(to_s3s_etag(&staged_etag)), + ..Default::default() + }]), + })) + .build() + .expect("complete multipart input should build"); + let completed = DefaultMultipartUsecase::from_global() + .execute_complete_multipart_upload(build_request(input, Method::POST)) + .await + .expect("staged part should remain completable after empty request rejection"); + assert!(completed.output.e_tag.is_some()); + let completed_info = store + .get_object_info( + completed + .output + .bucket + .as_deref() + .expect("complete response should include bucket"), + object, + &ObjectOptions::default(), + ) + .await + .expect("completed object should be readable"); + assert_eq!(completed_info.size, b"staged part".len() as i64); + } + #[tokio::test] async fn execute_complete_multipart_upload_allows_duplicate_part_numbers_by_using_last_occurrence() { let multipart_upload = CompletedMultipartUpload {