From 7929f22ec67abbf26f69e0934586646564a8f39a Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Thu, 3 Oct 2024 16:30:25 +0300 Subject: [PATCH 1/2] [#404] Add check of encoding type in object listing Signed-off-by: Marina Biryukova --- api/handler/object_list.go | 9 ++++ api/handler/object_list_test.go | 85 ++++++++++++++++++++++++--------- 2 files changed, 72 insertions(+), 22 deletions(-) diff --git a/api/handler/object_list.go b/api/handler/object_list.go index 5119707..502b8d2 100644 --- a/api/handler/object_list.go +++ b/api/handler/object_list.go @@ -4,6 +4,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" @@ -153,6 +154,10 @@ func parseListObjectArgs(reqInfo *middleware.ReqInfo) (*layer.ListObjectsParamsC res.Delimiter = queryValues.Get("delimiter") res.Encode = queryValues.Get("encoding-type") + if res.Encode != "" && strings.ToLower(res.Encode) != urlEncodingType { + return nil, errors.GetAPIError(errors.ErrInvalidEncodingMethod) + } + if queryValues.Get("max-keys") == "" { res.MaxKeys = maxObjectList } else if res.MaxKeys, err = strconv.Atoi(queryValues.Get("max-keys")); err != nil || res.MaxKeys < 0 { @@ -257,6 +262,10 @@ func parseListObjectVersionsRequest(reqInfo *middleware.ReqInfo) (*layer.ListObj res.Encode = queryValues.Get("encoding-type") res.VersionIDMarker = queryValues.Get("version-id-marker") + if res.Encode != "" && strings.ToLower(res.Encode) != urlEncodingType { + return nil, errors.GetAPIError(errors.ErrInvalidEncodingMethod) + } + if res.VersionIDMarker != "" && res.KeyMarker == "" { return nil, errors.GetAPIError(errors.VersionIDMarkerWithoutKeyMarker) } diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go index 6c9605e..5a13953 100644 --- a/api/handler/object_list_test.go +++ b/api/handler/object_list_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/http/httptest" "net/url" "sort" "strconv" @@ -14,6 +15,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/cache" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" + apierr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/encryption" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" @@ -755,6 +757,16 @@ func TestListObjectVersionsEncoding(t *testing.T) { require.Equal(t, 3, listResponse.MaxKeys) } +func TestListingsWithInvalidEncodingType(t *testing.T) { + hc := prepareHandlerContext(t) + bktName := "bucket-for-listing-invalid-encoding" + createTestBucket(hc, bktName) + + listObjectsVersionsErr(hc, bktName, "invalid", apierr.GetAPIError(apierr.ErrInvalidEncodingMethod)) + listObjectsV2Err(hc, bktName, "invalid", apierr.GetAPIError(apierr.ErrInvalidEncodingMethod)) + listObjectsV1Err(hc, bktName, "invalid", apierr.GetAPIError(apierr.ErrInvalidEncodingMethod)) +} + func checkVersionsNames(t *testing.T, versions *ListObjectsVersionsResponse, names []string) { for i, v := range versions.Version { require.Equal(t, names[i], v.Key) @@ -762,10 +774,19 @@ func checkVersionsNames(t *testing.T, versions *ListObjectsVersionsResponse, nam } func listObjectsV2(hc *handlerContext, bktName, prefix, delimiter, startAfter, continuationToken string, maxKeys int) *ListObjectsV2Response { - return listObjectsV2Ext(hc, bktName, prefix, delimiter, startAfter, continuationToken, "", maxKeys) + w := listObjectsV2Base(hc, bktName, prefix, delimiter, startAfter, continuationToken, "", maxKeys) + assertStatus(hc.t, w, http.StatusOK) + res := &ListObjectsV2Response{} + parseTestResponse(hc.t, w, res) + return res } -func listObjectsV2Ext(hc *handlerContext, bktName, prefix, delimiter, startAfter, continuationToken, encodingType string, maxKeys int) *ListObjectsV2Response { +func listObjectsV2Err(hc *handlerContext, bktName, encoding string, err apierr.Error) { + w := listObjectsV2Base(hc, bktName, "", "", "", "", encoding, -1) + assertS3Error(hc.t, w, err) +} + +func listObjectsV2Base(hc *handlerContext, bktName, prefix, delimiter, startAfter, continuationToken, encodingType string, maxKeys int) *httptest.ResponseRecorder { query := prepareCommonListObjectsQuery(prefix, delimiter, maxKeys) query.Add("fetch-owner", "true") if len(startAfter) != 0 { @@ -780,10 +801,7 @@ func listObjectsV2Ext(hc *handlerContext, bktName, prefix, delimiter, startAfter w, r := prepareTestFullRequest(hc, bktName, "", query, nil) hc.Handler().ListObjectsV2Handler(w, r) - assertStatus(hc.t, w, http.StatusOK) - res := &ListObjectsV2Response{} - parseTestResponse(hc.t, w, res) - return res + return w } func validateListV1(t *testing.T, tc *handlerContext, bktName, prefix, delimiter, marker string, maxKeys int, @@ -843,28 +861,54 @@ func prepareCommonListObjectsQuery(prefix, delimiter string, maxKeys int) url.Va } func listObjectsV1(hc *handlerContext, bktName, prefix, delimiter, marker string, maxKeys int) *ListObjectsV1Response { - query := prepareCommonListObjectsQuery(prefix, delimiter, maxKeys) - if len(marker) != 0 { - query.Add("marker", marker) - } - - w, r := prepareTestFullRequest(hc, bktName, "", query, nil) - hc.Handler().ListObjectsV1Handler(w, r) + w := listObjectsV1Base(hc, bktName, prefix, delimiter, marker, "", maxKeys) assertStatus(hc.t, w, http.StatusOK) res := &ListObjectsV1Response{} parseTestResponse(hc.t, w, res) return res } +func listObjectsV1Err(hc *handlerContext, bktName, encoding string, err apierr.Error) { + w := listObjectsV1Base(hc, bktName, "", "", "", encoding, -1) + assertS3Error(hc.t, w, err) +} + +func listObjectsV1Base(hc *handlerContext, bktName, prefix, delimiter, marker, encoding string, maxKeys int) *httptest.ResponseRecorder { + query := prepareCommonListObjectsQuery(prefix, delimiter, maxKeys) + if len(marker) != 0 { + query.Add("marker", marker) + } + if len(encoding) != 0 { + query.Add("encoding-type", encoding) + } + + w, r := prepareTestFullRequest(hc, bktName, "", query, nil) + hc.Handler().ListObjectsV1Handler(w, r) + return w +} + func listObjectsVersions(hc *handlerContext, bktName, prefix, delimiter, keyMarker, versionIDMarker string, maxKeys int) *ListObjectsVersionsResponse { - return listObjectsVersionsBase(hc, bktName, prefix, delimiter, keyMarker, versionIDMarker, maxKeys, false) + w := listObjectsVersionsBase(hc, bktName, prefix, delimiter, keyMarker, versionIDMarker, "", maxKeys) + assertStatus(hc.t, w, http.StatusOK) + res := &ListObjectsVersionsResponse{} + parseTestResponse(hc.t, w, res) + return res } func listObjectsVersionsURL(hc *handlerContext, bktName, prefix, delimiter, keyMarker, versionIDMarker string, maxKeys int) *ListObjectsVersionsResponse { - return listObjectsVersionsBase(hc, bktName, prefix, delimiter, keyMarker, versionIDMarker, maxKeys, true) + w := listObjectsVersionsBase(hc, bktName, prefix, delimiter, keyMarker, versionIDMarker, urlEncodingType, maxKeys) + assertStatus(hc.t, w, http.StatusOK) + res := &ListObjectsVersionsResponse{} + parseTestResponse(hc.t, w, res) + return res } -func listObjectsVersionsBase(hc *handlerContext, bktName, prefix, delimiter, keyMarker, versionIDMarker string, maxKeys int, encode bool) *ListObjectsVersionsResponse { +func listObjectsVersionsErr(hc *handlerContext, bktName, encoding string, err apierr.Error) { + w := listObjectsVersionsBase(hc, bktName, "", "", "", "", encoding, -1) + assertS3Error(hc.t, w, err) +} + +func listObjectsVersionsBase(hc *handlerContext, bktName, prefix, delimiter, keyMarker, versionIDMarker, encoding string, maxKeys int) *httptest.ResponseRecorder { query := prepareCommonListObjectsQuery(prefix, delimiter, maxKeys) if len(keyMarker) != 0 { query.Add("key-marker", keyMarker) @@ -872,14 +916,11 @@ func listObjectsVersionsBase(hc *handlerContext, bktName, prefix, delimiter, key if len(versionIDMarker) != 0 { query.Add("version-id-marker", versionIDMarker) } - if encode { - query.Add("encoding-type", "url") + if len(encoding) != 0 { + query.Add("encoding-type", encoding) } w, r := prepareTestFullRequest(hc, bktName, "", query, nil) hc.Handler().ListBucketObjectVersionsHandler(w, r) - assertStatus(hc.t, w, http.StatusOK) - res := &ListObjectsVersionsResponse{} - parseTestResponse(hc.t, w, res) - return res + return w } -- 2.45.2 From ca837771d27afcff23b6249ffdc79b2383819569 Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Thu, 3 Oct 2024 16:31:03 +0300 Subject: [PATCH 2/2] [#404] Fix using encoding type in multipart upload listing Signed-off-by: Marina Biryukova --- api/handler/multipart_upload.go | 16 ++++-- api/handler/multipart_upload_test.go | 84 ++++++++++++++++++++++++---- 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/api/handler/multipart_upload.go b/api/handler/multipart_upload.go index 17c592d..0a719dc 100644 --- a/api/handler/multipart_upload.go +++ b/api/handler/multipart_upload.go @@ -7,6 +7,7 @@ import ( "net/url" "path" "strconv" + "strings" "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" @@ -525,6 +526,11 @@ func (h *handler) ListMultipartUploadsHandler(w http.ResponseWriter, r *http.Req UploadIDMarker: queryValues.Get(uploadIDMarkerQueryName), } + if p.EncodingType != "" && strings.ToLower(p.EncodingType) != urlEncodingType { + h.logAndSendError(w, "invalid encoding type", reqInfo, errors.GetAPIError(errors.ErrInvalidEncodingMethod)) + return + } + list, err := h.obj.ListMultipartUploads(r.Context(), p) if err != nil { h.logAndSendError(w, "could not list multipart uploads", reqInfo, err) @@ -635,14 +641,14 @@ func encodeListMultipartUploadsToResponse(info *layer.ListMultipartUploadsInfo, res := ListMultipartUploadsResponse{ Bucket: params.Bkt.Name, CommonPrefixes: fillPrefixes(info.Prefixes, params.EncodingType), - Delimiter: params.Delimiter, + Delimiter: s3PathEncode(params.Delimiter, params.EncodingType), EncodingType: params.EncodingType, IsTruncated: info.IsTruncated, - KeyMarker: params.KeyMarker, + KeyMarker: s3PathEncode(params.KeyMarker, params.EncodingType), MaxUploads: params.MaxUploads, - NextKeyMarker: info.NextKeyMarker, + NextKeyMarker: s3PathEncode(info.NextKeyMarker, params.EncodingType), NextUploadIDMarker: info.NextUploadIDMarker, - Prefix: params.Prefix, + Prefix: s3PathEncode(params.Prefix, params.EncodingType), UploadIDMarker: params.UploadIDMarker, } @@ -654,7 +660,7 @@ func encodeListMultipartUploadsToResponse(info *layer.ListMultipartUploadsInfo, ID: u.Owner.String(), DisplayName: u.Owner.String(), }, - Key: u.Key, + Key: s3PathEncode(u.Key, params.EncodingType), Owner: Owner{ ID: u.Owner.String(), DisplayName: u.Owner.String(), diff --git a/api/handler/multipart_upload_test.go b/api/handler/multipart_upload_test.go index ddfe905..2831b3e 100644 --- a/api/handler/multipart_upload_test.go +++ b/api/handler/multipart_upload_test.go @@ -9,6 +9,7 @@ import ( "encoding/xml" "fmt" "net/http" + "net/http/httptest" "net/url" "strconv" "testing" @@ -252,14 +253,14 @@ func TestListMultipartUploads(t *testing.T) { }) t.Run("check max uploads", func(t *testing.T) { - listUploads := listMultipartUploadsBase(hc, bktName, "", "", "", "", 2) + listUploads := listMultipartUploads(hc, bktName, "", "", "", "", 2) require.Len(t, listUploads.Uploads, 2) require.Equal(t, uploadInfo1.UploadID, listUploads.Uploads[0].UploadID) require.Equal(t, uploadInfo2.UploadID, listUploads.Uploads[1].UploadID) }) t.Run("check prefix", func(t *testing.T) { - listUploads := listMultipartUploadsBase(hc, bktName, "/my", "", "", "", -1) + listUploads := listMultipartUploads(hc, bktName, "/my", "", "", "", -1) require.Len(t, listUploads.Uploads, 2) require.Equal(t, uploadInfo1.UploadID, listUploads.Uploads[0].UploadID) require.Equal(t, uploadInfo2.UploadID, listUploads.Uploads[1].UploadID) @@ -267,7 +268,7 @@ func TestListMultipartUploads(t *testing.T) { t.Run("check markers", func(t *testing.T) { t.Run("check only key-marker", func(t *testing.T) { - listUploads := listMultipartUploadsBase(hc, bktName, "", "", "", objName2, -1) + listUploads := listMultipartUploads(hc, bktName, "", "", "", objName2, -1) require.Len(t, listUploads.Uploads, 1) // If upload-id-marker is not specified, only the keys lexicographically greater than the specified key-marker will be included in the list. require.Equal(t, uploadInfo3.UploadID, listUploads.Uploads[0].UploadID) @@ -278,7 +279,7 @@ func TestListMultipartUploads(t *testing.T) { if uploadIDMarker > uploadInfo2.UploadID { uploadIDMarker = uploadInfo2.UploadID } - listUploads := listMultipartUploadsBase(hc, bktName, "", "", uploadIDMarker, "", -1) + listUploads := listMultipartUploads(hc, bktName, "", "", uploadIDMarker, "", -1) // If key-marker is not specified, the upload-id-marker parameter is ignored. require.Len(t, listUploads.Uploads, 3) }) @@ -286,7 +287,7 @@ func TestListMultipartUploads(t *testing.T) { t.Run("check key-marker along with upload-id-marker", func(t *testing.T) { uploadIDMarker := "00000000-0000-0000-0000-000000000000" - listUploads := listMultipartUploadsBase(hc, bktName, "", "", uploadIDMarker, objName3, -1) + listUploads := listMultipartUploads(hc, bktName, "", "", uploadIDMarker, objName3, -1) require.Len(t, listUploads.Uploads, 1) // If upload-id-marker is specified, any multipart uploads for a key equal to the key-marker might also be included, // provided those multipart uploads have upload IDs lexicographically greater than the specified upload-id-marker. @@ -653,6 +654,42 @@ func TestUploadPartWithNegativeContentLength(t *testing.T) { require.Equal(t, partSize, resp.ObjectParts.Parts[0].Size) } +func TestListMultipartUploadsEncoding(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName := "bucket-to-list-uploads-encoding" + createTestBucket(hc, bktName) + + listAllMultipartUploadsErr(hc, bktName, "invalid", apierr.GetAPIError(apierr.ErrInvalidEncodingMethod)) + + objects := []string{"foo()/bar", "foo()/bar/xyzzy", "asdf+b"} + for _, objName := range objects { + createMultipartUpload(hc, bktName, objName, nil) + } + + listResponse := listMultipartUploadsURL(hc, bktName, "foo(", ")", "", "", -1) + + require.Len(t, listResponse.CommonPrefixes, 1) + require.Equal(t, "foo%28%29", listResponse.CommonPrefixes[0].Prefix) + require.Equal(t, "foo%28", listResponse.Prefix) + require.Equal(t, "%29", listResponse.Delimiter) + require.Equal(t, "url", listResponse.EncodingType) + require.Equal(t, maxObjectList, listResponse.MaxUploads) + + listResponse = listMultipartUploads(hc, bktName, "", "", "", "", 1) + require.Empty(t, listResponse.EncodingType) + + listResponse = listMultipartUploadsURL(hc, bktName, "", "", "", listResponse.NextKeyMarker, 1) + + require.Len(t, listResponse.CommonPrefixes, 0) + require.Len(t, listResponse.Uploads, 1) + require.Equal(t, "foo%28%29/bar", listResponse.Uploads[0].Key) + require.Equal(t, "asdf%2Bb", listResponse.KeyMarker) + require.Equal(t, "foo%28%29/bar", listResponse.NextKeyMarker) + require.Equal(t, "url", listResponse.EncodingType) + require.Equal(t, 1, listResponse.MaxUploads) +} + func uploadPartCopy(hc *handlerContext, bktName, objName, uploadID string, num int, srcObj string, start, end int) *UploadPartCopyResponse { return uploadPartCopyBase(hc, bktName, objName, false, uploadID, num, srcObj, start, end) } @@ -678,16 +715,42 @@ func uploadPartCopyBase(hc *handlerContext, bktName, objName string, encrypted b return uploadPartCopyResponse } -func listAllMultipartUploads(hc *handlerContext, bktName string) *ListMultipartUploadsResponse { - return listMultipartUploadsBase(hc, bktName, "", "", "", "", -1) +func listMultipartUploads(hc *handlerContext, bktName, prefix, delimiter, uploadIDMarker, keyMarker string, maxUploads int) *ListMultipartUploadsResponse { + w := listMultipartUploadsBase(hc, bktName, prefix, delimiter, uploadIDMarker, keyMarker, "", maxUploads) + assertStatus(hc.t, w, http.StatusOK) + res := &ListMultipartUploadsResponse{} + parseTestResponse(hc.t, w, res) + return res } -func listMultipartUploadsBase(hc *handlerContext, bktName, prefix, delimiter, uploadIDMarker, keyMarker string, maxUploads int) *ListMultipartUploadsResponse { +func listMultipartUploadsURL(hc *handlerContext, bktName, prefix, delimiter, uploadIDMarker, keyMarker string, maxUploads int) *ListMultipartUploadsResponse { + w := listMultipartUploadsBase(hc, bktName, prefix, delimiter, uploadIDMarker, keyMarker, urlEncodingType, maxUploads) + assertStatus(hc.t, w, http.StatusOK) + res := &ListMultipartUploadsResponse{} + parseTestResponse(hc.t, w, res) + return res +} + +func listAllMultipartUploads(hc *handlerContext, bktName string) *ListMultipartUploadsResponse { + w := listMultipartUploadsBase(hc, bktName, "", "", "", "", "", -1) + assertStatus(hc.t, w, http.StatusOK) + res := &ListMultipartUploadsResponse{} + parseTestResponse(hc.t, w, res) + return res +} + +func listAllMultipartUploadsErr(hc *handlerContext, bktName, encoding string, err apierr.Error) { + w := listMultipartUploadsBase(hc, bktName, "", "", "", "", encoding, -1) + assertS3Error(hc.t, w, err) +} + +func listMultipartUploadsBase(hc *handlerContext, bktName, prefix, delimiter, uploadIDMarker, keyMarker, encoding string, maxUploads int) *httptest.ResponseRecorder { query := make(url.Values) query.Set(prefixQueryName, prefix) query.Set(delimiterQueryName, delimiter) query.Set(uploadIDMarkerQueryName, uploadIDMarker) query.Set(keyMarkerQueryName, keyMarker) + query.Set(encodingTypeQueryName, encoding) if maxUploads != -1 { query.Set(maxUploadsQueryName, strconv.Itoa(maxUploads)) } @@ -695,10 +758,7 @@ func listMultipartUploadsBase(hc *handlerContext, bktName, prefix, delimiter, up w, r := prepareTestRequestWithQuery(hc, bktName, "", query, nil) hc.Handler().ListMultipartUploadsHandler(w, r) - listPartsResponse := &ListMultipartUploadsResponse{} - readResponse(hc.t, w, http.StatusOK, listPartsResponse) - - return listPartsResponse + return w } func listParts(hc *handlerContext, bktName, objName string, uploadID, partNumberMarker string, status int) *ListPartsResponse { -- 2.45.2