From 39fc7aa3ee4096c38f31aa380c54738bc104a141 Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Thu, 3 Oct 2024 16:31:03 +0300 Subject: [PATCH] [#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 {