fix: reject ListMultipartUploads requests that also carry uploadId

* 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

* fix: reject uploads with uploadId for every bucket and object operation

Requests carrying both the uploads and uploadId query parameters were
only rejected for a bucket GET, so DELETE, PUT, POST and HEAD on the
bucket or object paths still ran real operations. S3 rejects the
combination for every method on both paths, before authentication and
before checking that the bucket exists.

Register the error route for all methods on the bucket and object
routers right after the CORS middleware is set up, ahead of every other
route, and cover each method and path with an integration test.
This commit is contained in:
dualfroz committed 2026-10-05 11:18:49 -07:00
1 parent cc9a86ef08
commit cd3095d5b7
6 files changed
+87 -3

No files matched your search

+25
View File
@@ -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 {
+42
View File
@@ -195,6 +195,48 @@ func RouterCopySourceNotAllowed(s *S3Conf) error {
})
}
func RouterUploadsWithUploadId(s *S3Conf) error {
testName := "RouterUploadsWithUploadId"
return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error {
for _, method := range []string{
http.MethodGet,
http.MethodHead,
http.MethodPut,
http.MethodPost,
http.MethodDelete,
} {
for _, path := range []string{
"/bucket?uploads&uploadId=something",
"/bucket/object?uploads&uploadId=something",
} {
req, err := http.NewRequest(method, s.endpoint+path, nil)
if err != nil {
return fmt.Errorf("failed to make %s request to %s", method, path)
}
resp, err := s.httpClient.Do(req)
if err != nil {
return fmt.Errorf("failed to send %s request to %s", method, path)
}
if method == http.MethodHead {
// for head requests only check the status code
if resp.StatusCode != http.StatusBadRequest {
return fmt.Errorf("expected 400 status code for HEAD %s request, instead got %v", path, resp.StatusCode)
}
continue
}
if err := checkHTTPResponseApiErr(resp, s3err.GetInvalidArgumentErr(s3err.InvalidArgUploadIdWithUploads, "uploadId")); err != nil {
return fmt.Errorf("%s %s: %w", method, path, err)
}
}
}
return nil
})
}
func RouterListVersionsWithKey(s *S3Conf) error {
testName := "RouterListVersionsWithKey"
return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error {
+4
View File
@@ -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)
@@ -2190,6 +2191,7 @@ func TestRouter(ts *TestState) {
ts.Run(RouterGetUploadsWithKey)
ts.Run(RouterCopySourceNotAllowed)
ts.Run(RouterListVersionsWithKey)
ts.Run(RouterUploadsWithUploadId)
ts.Run(RouterObjectAnnotationNotImplemented)
ts.Run(RouterRenameObjectNotImplemented)
ts.Run(RouterObjectTorrentMethodNotAllowed)
@@ -3288,6 +3290,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,
@@ -3792,6 +3795,7 @@ func GetIntTests() IntTests {
"RouterGetUploadsWithKey": RouterGetUploadsWithKey,
"RouterCopySourceNotAllowed": RouterCopySourceNotAllowed,
"RouterListVersionsWithKey": RouterListVersionsWithKey,
"RouterUploadsWithUploadId": RouterUploadsWithUploadId,
"RouterObjectAnnotationNotImplemented": RouterObjectAnnotationNotImplemented,
"RouterRenameObjectNotImplemented": RouterRenameObjectNotImplemented,
"RouterObjectTorrentMethodNotAllowed": RouterObjectTorrentMethodNotAllowed,