diff --git a/s3api/router.go b/s3api/router.go index 3ec46da..fabeb52 100644 --- a/s3api/router.go +++ b/s3api/router.go @@ -102,6 +102,17 @@ func (sa *S3ApiRouter) Init() { objectRouter := sa.app.Group("/:bucket/*") applyBucketCORS := middlewares.ApplyBucketCORS(sa.be, middlewares.BucketFromPath, sa.corsAllowOrigin) + // '?uploads' together with '?uploadId' is rejected for every + // bucket and object operation + for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodPut, http.MethodPost, http.MethodDelete} { + for _, router := range []fiber.Router{bucketRouter, objectRouter} { + router.Add([]string{method}, "", + middlewares.MatchQueryArgs("uploads", "uploadId"), + controllers.ProcessHandlers(ctrl.HandleErrorRoute(s3err.GetInvalidArgumentErr(s3err.InvalidArgUploadIdWithUploads, "uploadId")), metrics.ActionUndetected, services), + ) + } + } + // PUT bucket operations bucketRouter.Put("", middlewares.MatchQueryArgs("tagging"), 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/general.go b/tests/integration/general.go index 50ff019..ba22f10 100644 --- a/tests/integration/general.go +++ b/tests/integration/general.go @@ -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 { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index bd053a9..96789c9 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) @@ -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, 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"