fix(s3): report malformed multipart upload IDs as NoSuchUpload (#9147)

UploadPart, UploadPartCopy, ListParts and CompleteMultipartUpload returned 400 InvalidArgument for a malformed upload ID, while AbortMultipartUpload returned 404 NoSuchUpload. AWS S3 documents NoSuchUpload for an upload ID that might be invalid.

Report MalformedUploadID as NoSuchUpload through one helper used by all operations that address an existing upload, replacing the Abort-only special case. ListMultipartUploads keeps InvalidArgument for a malformed upload-id-marker.

Add the multipart_upload_id_test e2e module to the e2e-smoke profile. It asserts 404 NoSuchUpload from every operation that addresses an existing upload, for a malformed upload ID and for an aborted one, and updates the e2e-smoke and Linux e2e-full selection digests.

Co-authored-by: Karl Bastarache <bastarachek@google.com>
Co-authored-by: Hauser Zhi <housemecn@gmail.com>
This commit is contained in:
KarlEmm
2026-11-04 16:25:00 +00:00
committed by GitHub
parent 4d46bd25f8
commit 7babc7d621
6 changed files with 220 additions and 20 deletions
+1 -1
View File
@@ -1,2 +1,2 @@
sha256-darwin=7c705942d3a9e583ebaa85845daa21c4f81fdbf36def24db931aaef38636c53e
sha256-linux=5c33dc395233671cb77452f8fb12900ecdbeb368632222b96835d310bce1aab8
sha256-linux=01e1f5569be60674e30d9e30a9961c5c53a6cea6ca1f7913218f2adf7291d574
+1 -1
View File
@@ -1 +1 @@
sha256=00f8a56b75990b487a594b97c36610b94476a6944ba00473e37187ea654d42bf
sha256=f794c004e5952147c79dadbe6b7c84804bc986f34a8142731fac8864004e5130
+1 -1
View File
@@ -701,7 +701,7 @@ test-group = 'ecstore-serial-flaky'
[profile.e2e-smoke]
default-filter = """
package(e2e_test) & (
test(/^(delete_marker_migration_semantics|version_id_regression|list_objects_v2_pagination|list_object_versions_regression|list_objects_duplicates|list_buckets_double_slash|list_buckets_auth|list_buckets_iam_filter|leading_slash_key|special_chars|create_bucket_region|delete_objects_versioning|head_object_consistency|head_object_range|copy_object_metadata|copy_object_tagging|copy_source_invalid_date|content_encoding|compression|multipart_storage_class|storage_class_capability|ssec_copy|anonymous_access|bucket_policy_check|presigned_negative|negative_sigv4|admin_auth|notification_webhook|tls_hot_reload|console_smoke|admin_iam_crud|admin_pools|sts_query_compat|data_usage)_test::|^fake_s3_target::/)
test(/^(delete_marker_migration_semantics|version_id_regression|list_objects_v2_pagination|list_object_versions_regression|list_objects_duplicates|list_buckets_double_slash|list_buckets_auth|list_buckets_iam_filter|leading_slash_key|special_chars|create_bucket_region|delete_objects_versioning|head_object_consistency|head_object_range|copy_object_metadata|copy_object_tagging|copy_source_invalid_date|content_encoding|compression|multipart_storage_class|multipart_upload_id|storage_class_capability|ssec_copy|anonymous_access|bucket_policy_check|presigned_negative|negative_sigv4|admin_auth|notification_webhook|tls_hot_reload|console_smoke|admin_iam_crud|admin_pools|sts_query_compat|data_usage)_test::|^fake_s3_target::/)
| test(/^replication_extension_test::(test_replication_check_succeeds_with_remote_target|test_replication_check_rejects_target_without_object_lock|test_set_remote_target_rejects_unversioned_source_bucket|test_replication_check_rejects_unversioned_source_bucket|test_replication_check_rejects_missing_replication_config|test_replication_check_rejects_invalid_bucket|test_set_remote_target_rejects_same_bucket_on_same_deployment|test_set_remote_target_rejects_unversioned_target_bucket|test_set_remote_target_update_requires_arn|test_set_remote_target_update_rejects_missing_target|test_set_remote_target_rejects_invalid_target_url|test_set_remote_target_rejects_self_signed_https_target_without_skip_tls_verify|test_set_remote_target_rejects_private_ca_https_target_without_ca_cert_pem|test_list_remote_targets_rejects_empty_bucket|test_list_remote_targets_rejects_invalid_bucket|test_remove_remote_target_rejects_missing_target|test_remove_remote_target_rejects_missing_arn|test_remove_remote_target_rejects_invalid_bucket|test_remove_remote_target_rejects_target_used_by_replication|test_delete_bucket_replication_removes_remote_target)$/)
| test(/^reliant::lifecycle::/)
| test(/^conditional_delete_objects_test::/)
+3
View File
@@ -283,6 +283,9 @@ mod ssec_copy_test;
#[cfg(test)]
mod multipart_storage_class_test;
#[cfg(test)]
mod multipart_upload_id_test;
#[cfg(test)]
mod storage_class_capability_test;
@@ -0,0 +1,175 @@
// Copyright 2026 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.
//! Multipart upload ID regression tests.
//!
//! Every operation that addresses an existing multipart upload must report an
//! upload ID that does not name a live upload as `404 NoSuchUpload`, whether
//! the ID is malformed or is a well-formed ID of an aborted upload.
#[cfg(test)]
mod tests {
use crate::common::{RustFSTestEnvironment, init_logging};
use aws_sdk_s3::Client;
use aws_sdk_s3::error::{ProvideErrorMetadata, SdkError};
use aws_sdk_s3::primitives::ByteStream;
use aws_sdk_s3::types::{CompletedMultipartUpload, CompletedPart};
type TestResult = Result<(), Box<dyn std::error::Error + Send + Sync>>;
const BUCKET: &str = "multipart-upload-id";
const KEY: &str = "target.bin";
const COPY_SOURCE_KEY: &str = "copy-source.bin";
// RustFS upload IDs are URL-safe base64; '.' is outside that alphabet, so
// the object layer rejects this ID as malformed instead of looking it up.
const MALFORMED_UPLOAD_ID: &str = "malformed.upload.id";
const PART_NUMBER: i32 = 1;
const PART_BODY: &[u8] = b"multipart upload id regression part";
const PART_ETAG: &str = "\"00000000000000000000000000000000\"";
const NO_SUCH_UPLOAD_STATUS: u16 = 404;
const NO_SUCH_UPLOAD_CODE: &str = "NoSuchUpload";
const OBJECT_NOT_FOUND_STATUS: u16 = 404;
/// Describes how `result` differs from `404 NoSuchUpload`, or returns `None` when it matches.
fn no_such_upload_mismatch<T, E>(operation: &str, result: Result<T, SdkError<E>>) -> Option<String>
where
E: ProvideErrorMetadata + std::fmt::Debug,
{
let Err(error) = result else {
return Some(format!("{operation}: succeeded"));
};
let status = error.raw_response().map(|response| response.status().as_u16());
let code = error.as_service_error().and_then(ProvideErrorMetadata::code);
(status != Some(NO_SUCH_UPLOAD_STATUS) || code != Some(NO_SUCH_UPLOAD_CODE))
.then(|| format!("{operation}: status {status:?}, code {code:?}: {error:?}"))
}
async fn start_server_with_copy_source() -> Result<(RustFSTestEnvironment, Client), Box<dyn std::error::Error + Send + Sync>>
{
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
env.start_rustfs_server(Vec::new()).await?;
env.create_test_bucket(BUCKET).await?;
let client = env.create_s3_client();
client
.put_object()
.bucket(BUCKET)
.key(COPY_SOURCE_KEY)
.body(ByteStream::from_static(PART_BODY))
.send()
.await?;
Ok((env, client))
}
/// Sends every operation that addresses an existing upload and asserts that all of them report `404 NoSuchUpload`.
async fn assert_every_upload_operation_reports_no_such_upload(client: &Client, upload_id: &str) {
let upload_part = client
.upload_part()
.bucket(BUCKET)
.key(KEY)
.upload_id(upload_id)
.part_number(PART_NUMBER)
.body(ByteStream::from_static(PART_BODY))
.send()
.await;
let upload_part_copy = client
.upload_part_copy()
.bucket(BUCKET)
.key(KEY)
.upload_id(upload_id)
.part_number(PART_NUMBER)
.copy_source(format!("/{BUCKET}/{COPY_SOURCE_KEY}"))
.send()
.await;
let list_parts = client.list_parts().bucket(BUCKET).key(KEY).upload_id(upload_id).send().await;
let complete = client
.complete_multipart_upload()
.bucket(BUCKET)
.key(KEY)
.upload_id(upload_id)
.multipart_upload(
CompletedMultipartUpload::builder()
.parts(CompletedPart::builder().part_number(PART_NUMBER).e_tag(PART_ETAG).build())
.build(),
)
.send()
.await;
let abort = client
.abort_multipart_upload()
.bucket(BUCKET)
.key(KEY)
.upload_id(upload_id)
.send()
.await;
let mismatches: Vec<String> = [
no_such_upload_mismatch("UploadPart", upload_part),
no_such_upload_mismatch("UploadPartCopy", upload_part_copy),
no_such_upload_mismatch("ListParts", list_parts),
no_such_upload_mismatch("CompleteMultipartUpload", complete),
no_such_upload_mismatch("AbortMultipartUpload", abort),
]
.into_iter()
.flatten()
.collect();
assert!(
mismatches.is_empty(),
"upload ID {upload_id:?}: operations not reporting {NO_SUCH_UPLOAD_STATUS} {NO_SUCH_UPLOAD_CODE}:\n{}",
mismatches.join("\n")
);
}
/// Asserts that the rejected operations left neither a multipart upload nor an object under `KEY`.
async fn assert_no_upload_or_object_published(client: &Client) -> TestResult {
let uploads = client.list_multipart_uploads().bucket(BUCKET).prefix(KEY).send().await?;
assert!(uploads.uploads().is_empty(), "rejected operations must not leave an upload: {uploads:?}");
let head = client.head_object().bucket(BUCKET).key(KEY).send().await;
let status = head
.as_ref()
.err()
.and_then(|error| error.raw_response())
.map(|response| response.status().as_u16());
assert_eq!(
status,
Some(OBJECT_NOT_FOUND_STATUS),
"rejected operations must not publish {KEY}: {head:?}"
);
Ok(())
}
#[tokio::test]
async fn malformed_upload_id_reports_no_such_upload_on_every_upload_operation() -> TestResult {
let (_env, client) = start_server_with_copy_source().await?;
assert_every_upload_operation_reports_no_such_upload(&client, MALFORMED_UPLOAD_ID).await;
assert_no_upload_or_object_published(&client).await
}
#[tokio::test]
async fn aborted_upload_id_reports_no_such_upload_on_every_upload_operation() -> TestResult {
let (_env, client) = start_server_with_copy_source().await?;
let created = client.create_multipart_upload().bucket(BUCKET).key(KEY).send().await?;
let upload_id = created.upload_id().ok_or("CreateMultipartUpload returned no upload ID")?;
client
.abort_multipart_upload()
.bucket(BUCKET)
.key(KEY)
.upload_id(upload_id)
.send()
.await?;
assert_every_upload_operation_reports_no_such_upload(&client, upload_id).await;
assert_no_upload_or_object_published(&client).await
}
}
+39 -17
View File
@@ -551,12 +551,9 @@ impl DefaultMultipartUsecase {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
// Special handling for abort_multipart_upload: Per AWS S3 API specification, this operation
// should return NoSuchUpload (404) when the upload_id doesn't exist, even if the format
// appears invalid. This differs from other multipart operations (upload_part, list_parts,
// complete_multipart_upload) which return InvalidArgument for malformed upload_ids.
// The lenient validation matches AWS S3 behavior where format validation is relaxed for
// abort operations to avoid leaking information about upload_id format requirements.
// Per the AWS S3 API reference, a malformed upload ID is reported as
// NoSuchUpload (404), the same as an upload that was aborted or
// completed. See multipart_upload_api_error.
match store
.abort_multipart_upload(bucket.as_str(), key.as_str(), upload_id.as_str(), &opts)
.await
@@ -569,13 +566,7 @@ impl DefaultMultipartUsecase {
);
Ok(S3Response::new(AbortMultipartUploadOutput { ..Default::default() }))
}
Err(err) => {
// Convert MalformedUploadID to NoSuchUpload for S3 API compatibility
if matches!(err, StorageError::MalformedUploadID(_)) {
return Err(S3Error::new(S3ErrorCode::NoSuchUpload));
}
Err(ApiError::from(err).into())
}
Err(err) => Err(multipart_upload_api_error(err)),
}
}
@@ -700,7 +691,7 @@ impl DefaultMultipartUsecase {
let multipart_info = match store.get_multipart_info(&bucket, &key, &upload_id, &opts).await {
Ok(info) => Some(info),
Err(err) if is_err_invalid_upload_id(&err) => None,
Err(err) => return Err(ApiError::from(err).into()),
Err(err) => return Err(multipart_upload_api_error(err)),
};
// A ciphertext-passthrough session stores encrypted parts verbatim and
// completes without the customer key (the replication client has none),
@@ -1283,7 +1274,7 @@ impl DefaultMultipartUsecase {
let fi = store
.get_multipart_info(&bucket, &key, &upload_id, &opts)
.await
.map_err(ApiError::from)?;
.map_err(multipart_upload_api_error)?;
let max_total_object_size = multipart_max_total_object_size(&fi.user_defined)?;
let mut size = require_upload_part_size(size, max_total_object_size.is_some())?;
if let Some(limit) = max_total_object_size
@@ -1678,7 +1669,7 @@ impl DefaultMultipartUsecase {
let res = store
.list_object_parts(&bucket, &key, &upload_id, params.part_number_marker, params.max_parts, &opts)
.await
.map_err(ApiError::from)?;
.map_err(multipart_upload_api_error)?;
Ok(S3Response::new(build_list_parts_output(res)))
}
@@ -1795,7 +1786,7 @@ impl DefaultMultipartUsecase {
let mp_info = store
.get_multipart_info(&bucket, &key, &upload_id, &dst_opts)
.await
.map_err(ApiError::from)?;
.map_err(multipart_upload_api_error)?;
let destination_size_limit = multipart_max_total_object_size(&mp_info.user_defined)?;
EncryptionRequest {
bucket: &bucket,
@@ -2124,6 +2115,21 @@ fn normalize_presigned_part_checksums(req: &mut S3Request<UploadPartInput>) -> S
Ok(())
}
/// Maps a storage error from an operation that addresses an existing multipart
/// upload (UploadPart, UploadPartCopy, ListParts, CompleteMultipartUpload and
/// AbortMultipartUpload) to an S3 error.
///
/// A malformed upload ID cannot name an existing upload, so it is reported as
/// `NoSuchUpload` (404), which the AWS S3 error reference describes as "The
/// upload ID might be invalid, or the multipart upload might have been aborted
/// or completed".
fn multipart_upload_api_error(err: StorageError) -> S3Error {
if matches!(err, StorageError::MalformedUploadID(_)) {
return S3Error::new(S3ErrorCode::NoSuchUpload);
}
ApiError::from(err).into()
}
#[cfg(test)]
mod tests {
use super::*;
@@ -2797,6 +2803,22 @@ mod tests {
assert!(multipart_max_total_object_size(&metadata).is_err());
}
#[test]
fn multipart_upload_api_error_reports_malformed_upload_id_as_no_such_upload() {
let malformed = multipart_upload_api_error(StorageError::MalformedUploadID("bucket/object-not-an-id".to_string()));
assert_eq!(malformed.code(), &S3ErrorCode::NoSuchUpload);
let unknown = multipart_upload_api_error(StorageError::InvalidUploadID(
"bucket".to_string(),
"object".to_string(),
"dXBsb2FkLWlk".to_string(),
));
assert_eq!(unknown.code(), &S3ErrorCode::NoSuchUpload);
let other = multipart_upload_api_error(StorageError::ObjectNotFound("bucket".to_string(), "object".to_string()));
assert_eq!(other.code(), &S3ErrorCode::NoSuchKey);
}
#[tokio::test]
async fn execute_complete_multipart_upload_rejects_missing_parts_payload() {
let input = CompleteMultipartUploadInput::builder()