From 47ec1255892e4452aa6ad7fb82d1222de5fdde33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= <anzhengchao@gmail.com>
Date: Wed, 21 Jan 2026 20:45:18 +0800
Subject: [PATCH] fix(s3): return NoSuchUpload for abort_multipart_upload when
 upload_id not found (#1569)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: 安正超 <anzhengchao@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---
 rustfs/src/error.rs                    |  8 +++++++-
 rustfs/src/storage/ecfs.rs             | 20 +++++++++++++++++---
 scripts/s3-tests/implemented_tests.txt |  1 +
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/rustfs/src/error.rs b/rustfs/src/error.rs
index dabf7d7ae..56a52a5e4 100644
--- a/rustfs/src/error.rs
+++ b/rustfs/src/error.rs
@@ -219,6 +219,7 @@ impl From<StorageError> for ApiError {
             StorageError::SlowDown => S3ErrorCode::SlowDown,
             StorageError::PrefixAccessDenied(_, _) => S3ErrorCode::AccessDenied,
             StorageError::InvalidUploadIDKeyCombination(_, _) => S3ErrorCode::InvalidArgument,
+            StorageError::MalformedUploadID(_) => S3ErrorCode::InvalidArgument,
             StorageError::ObjectNameTooLong(_, _) => S3ErrorCode::InvalidArgument,
             StorageError::ObjectNamePrefixAsSlash(_, _) => S3ErrorCode::InvalidArgument,
             StorageError::ObjectNotFound(_, _) => S3ErrorCode::NoSuchKey,
@@ -227,7 +228,7 @@ impl From<StorageError> for ApiError {
             StorageError::FileNotFound => S3ErrorCode::NoSuchKey,
             StorageError::FileVersionNotFound => S3ErrorCode::NoSuchVersion,
             StorageError::VersionNotFound(_, _, _) => S3ErrorCode::NoSuchVersion,
-            StorageError::InvalidUploadID(_, _, _) => S3ErrorCode::InvalidPart,
+            StorageError::InvalidUploadID(_, _, _) => S3ErrorCode::NoSuchUpload,
             StorageError::InvalidVersionID(_, _, _) => S3ErrorCode::InvalidArgument,
             StorageError::DataMovementOverwriteErr(_, _, _) => S3ErrorCode::InvalidArgument,
             StorageError::ObjectExistsAsDirectory(_, _) => S3ErrorCode::InvalidArgument,
@@ -415,6 +416,11 @@ mod tests {
             (StorageError::VolumeNotFound, S3ErrorCode::NoSuchBucket),
             (StorageError::FileNotFound, S3ErrorCode::NoSuchKey),
             (StorageError::FileVersionNotFound, S3ErrorCode::NoSuchVersion),
+            (StorageError::MalformedUploadID("test".into()), S3ErrorCode::InvalidArgument),
+            (
+                StorageError::InvalidUploadID("bucket".into(), "object".into(), "uploadid".into()),
+                S3ErrorCode::NoSuchUpload,
+            ),
         ];
 
         for (storage_error, expected_code) in test_cases {
diff --git a/rustfs/src/storage/ecfs.rs b/rustfs/src/storage/ecfs.rs
index cf0a27c7e..a3e846c36 100644
--- a/rustfs/src/storage/ecfs.rs
+++ b/rustfs/src/storage/ecfs.rs
@@ -4902,11 +4902,25 @@ impl S3 for FS {
 
         let opts = &ObjectOptions::default();
 
-        store
+        // 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.
+        match store
             .abort_multipart_upload(bucket.as_str(), key.as_str(), upload_id.as_str(), opts)
             .await
-            .map_err(ApiError::from)?;
-        Ok(S3Response::new(AbortMultipartUploadOutput { ..Default::default() }))
+        {
+            Ok(_) => 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())
+            }
+        }
     }
 
     #[instrument(level = "debug", skip(self))]
diff --git a/scripts/s3-tests/implemented_tests.txt b/scripts/s3-tests/implemented_tests.txt
index a5122b4dc..3eb8933a0 100644
--- a/scripts/s3-tests/implemented_tests.txt
+++ b/scripts/s3-tests/implemented_tests.txt
@@ -138,3 +138,4 @@ test_ranged_request_empty_object
 test_ranged_request_invalid_range
 test_set_multipart_tagging
 test_upload_part_copy_percent_encoded_key
+test_api_error_from_storage_error_mappings