From 5f0e2e55baf07dd6587feab036f75f633e0d6090 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 2 Oct 2026 16:01:08 +0200 Subject: [PATCH] fix: reject ListMultipartUploads requests that also carry uploadId A bucket GET with both the uploads and uploadId query parameters was routed to ListMultipartUploads and answered with the upload listing. S3 rejects the combination with 400 InvalidArgument, "Conflicting query string parameters: uploadId, uploads". Add an error route for the combination ahead of ListMultipartUploads, the way the object level '?uploads' request is rejected, and enable the REST test that covers it. Fixes #2155 --- s3api/router.go | 5 +++++ s3err/invalid-argument.go | 5 +++++ tests/integration/ListMultipartUploads.go | 25 +++++++++++++++++++++++ tests/integration/group-tests.go | 2 ++ tests/test_rest_list_multipart_uploads.sh | 3 --- 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/s3api/router.go b/s3api/router.go index 3ec46da..a20cb8a 100644 --- a/s3api/router.go +++ b/s3api/router.go @@ -725,6 +725,11 @@ func (sa *S3ApiRouter) Init() { middlewares.VerifyV4Signature(sa.root, sa.iam, sa.region, false, false, false), middlewares.ParseAcl(sa.be), )) + // bucket operation with both '?uploads' and '?uploadId' is rejected + bucketRouter.Get("", + middlewares.MatchQueryArgs("uploads", "uploadId"), + controllers.ProcessHandlers(ctrl.HandleErrorRoute(s3err.GetInvalidArgumentErr(s3err.InvalidArgUploadIdWithUploads, "uploadId")), metrics.ActionUndetected, services), + ) bucketRouter.Get("", middlewares.MatchQueryArgs("uploads"), controllers.ProcessHandlers( diff --git a/s3err/invalid-argument.go b/s3err/invalid-argument.go index e814bfb..7bf61cd 100644 --- a/s3err/invalid-argument.go +++ b/s3err/invalid-argument.go @@ -73,6 +73,7 @@ const ( InvalidArgSSECIncompatibleEncryption InvalidArgSSEInvalidEncryptionMethod InvalidArgSSECRequiresTLS + InvalidArgUploadIdWithUploads ) var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ @@ -212,6 +213,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{ Description: "Invalid uploadId marker", ArgumentName: "upload-id-marker", }, + InvalidArgUploadIdWithUploads: { + Description: "Conflicting query string parameters: uploadId, uploads", + ArgumentName: "ResourceType", + }, InvalidArgCannedAcl: { Description: "", ArgumentName: "x-amz-acl", diff --git a/tests/integration/ListMultipartUploads.go b/tests/integration/ListMultipartUploads.go index f163875..07cb9fa 100644 --- a/tests/integration/ListMultipartUploads.go +++ b/tests/integration/ListMultipartUploads.go @@ -17,6 +17,7 @@ package integration import ( "context" "fmt" + "net/http" "sort" "strings" "time" @@ -579,6 +580,30 @@ func ListMultipartUploads_delimiter_no_matches(s *S3Conf) error { }) } +func ListMultipartUploads_with_upload_id(s *S3Conf) error { + testName := "ListMultipartUploads_with_upload_id" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + out, err := createMp(s3client, bucket, "my-obj") + if err != nil { + return err + } + + req, err := createSignedReq(http.MethodGet, s.endpoint, + fmt.Sprintf("%v?uploads&uploadId=%v", bucket, *out.UploadId), + s.awsID, s.awsSecret, "s3", s.awsRegion, "", nil, time.Now(), nil) + if err != nil { + return err + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return err + } + + return checkHTTPResponseApiErr(resp, s3err.GetInvalidArgumentErr(s3err.InvalidArgUploadIdWithUploads, "uploadId")) + }) +} + func ListMultipartUploads_with_checksums(s *S3Conf) error { testName := "ListMultipartUploads_with_checksums" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index bd053a9..7dd4b5e 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -554,6 +554,7 @@ func TestListMultipartUploads(ts *TestState) { ts.Run(ListMultipartUploads_prefix) ts.Run(ListMultipartUploads_both_delimiter_and_prefix) ts.Run(ListMultipartUploads_delimiter_no_matches) + ts.Run(ListMultipartUploads_with_upload_id) //TODO: remove the condition after implementing checksums in azure if !ts.conf.azureTests { ts.Run(ListMultipartUploads_with_checksums) @@ -3288,6 +3289,7 @@ func GetIntTests() IntTests { "ListMultipartUploads_delimiter_truncated": ListMultipartUploads_delimiter_truncated, "ListMultipartUploads_prefix": ListMultipartUploads_prefix, "ListMultipartUploads_both_delimiter_and_prefix": ListMultipartUploads_both_delimiter_and_prefix, + "ListMultipartUploads_with_upload_id": ListMultipartUploads_with_upload_id, "ListMultipartUploads_with_checksums": ListMultipartUploads_with_checksums, "AbortMultipartUpload_non_existing_bucket": AbortMultipartUpload_non_existing_bucket, "AbortMultipartUpload_incorrect_uploadId": AbortMultipartUpload_incorrect_uploadId, diff --git a/tests/test_rest_list_multipart_uploads.sh b/tests/test_rest_list_multipart_uploads.sh index 6038dad..4689ad0 100644 --- a/tests/test_rest_list_multipart_uploads.sh +++ b/tests/test_rest_list_multipart_uploads.sh @@ -215,9 +215,6 @@ source ./tests/setup.sh # tags: curl,ListMultipartUploads,multipart,uploadId,invalid-query @test "REST - ListMultipartUploads - uploads/uploadId combo" { - if [ "$DIRECT" != "true" ]; then - skip "https://github.com/versity/versitygw/issues/2155" - fi run setup_bucket_and_file_v3 "$BUCKET_ONE_NAME" assert_success read -r bucket_name test_file <<< "$output"