From 12cf29aed2de48db4b365336cbdb20ee71c20524 Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Fri, 15 Sep 2023 18:08:02 +0300 Subject: [PATCH] [#207] Fix part-number-marker handling Signed-off-by: Marina Biryukova --- api/handler/handlers_test.go | 6 ++-- api/handler/multipart_upload.go | 2 +- api/handler/multipart_upload_test.go | 47 ++++++++++++++++++++++++---- api/layer/multipart_upload.go | 4 +++ 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index f0ebf48b..c864f5e9 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -323,6 +323,8 @@ func assertStatus(t *testing.T, w *httptest.ResponseRecorder, status int) { func readResponse(t *testing.T, w *httptest.ResponseRecorder, status int, model interface{}) { assertStatus(t, w, status) - err := xml.NewDecoder(w.Result().Body).Decode(model) - require.NoError(t, err) + if status == http.StatusOK { + err := xml.NewDecoder(w.Result().Body).Decode(model) + require.NoError(t, err) + } } diff --git a/api/handler/multipart_upload.go b/api/handler/multipart_upload.go index 5e2ac398..04ec8b22 100644 --- a/api/handler/multipart_upload.go +++ b/api/handler/multipart_upload.go @@ -600,7 +600,7 @@ func (h *handler) ListPartsHandler(w http.ResponseWriter, r *http.Request) { } if queryValues.Get("part-number-marker") != "" { - if partNumberMarker, err = strconv.Atoi(queryValues.Get("part-number-marker")); err != nil || partNumberMarker <= 0 { + if partNumberMarker, err = strconv.Atoi(queryValues.Get("part-number-marker")); err != nil || partNumberMarker < 0 { h.logAndSendError(w, "invalid PartNumberMarker", reqInfo, err, additional...) return } diff --git a/api/handler/multipart_upload_test.go b/api/handler/multipart_upload_test.go index 3f288690..16921c8b 100644 --- a/api/handler/multipart_upload_test.go +++ b/api/handler/multipart_upload_test.go @@ -16,6 +16,10 @@ import ( "github.com/stretchr/testify/require" ) +const ( + partNumberMarkerQuery = "part-number-marker" +) + func TestPeriodicWriter(t *testing.T) { const dur = 100 * time.Millisecond const whitespaces = 8 @@ -80,7 +84,7 @@ func TestMultipartReUploadPart(t *testing.T) { etag1, _ := uploadPart(hc, bktName, objName, uploadInfo.UploadID, 1, partSizeLast) etag2, _ := uploadPart(hc, bktName, objName, uploadInfo.UploadID, 2, partSizeFirst) - list := listParts(hc, bktName, objName, uploadInfo.UploadID) + list := listParts(hc, bktName, objName, uploadInfo.UploadID, "0", http.StatusOK) require.Len(t, list.Parts, 2) require.Equal(t, etag1, list.Parts[0].ETag) require.Equal(t, etag2, list.Parts[1].ETag) @@ -91,7 +95,7 @@ func TestMultipartReUploadPart(t *testing.T) { etag1, data1 := uploadPart(hc, bktName, objName, uploadInfo.UploadID, 1, partSizeFirst) etag2, data2 := uploadPart(hc, bktName, objName, uploadInfo.UploadID, 2, partSizeLast) - list = listParts(hc, bktName, objName, uploadInfo.UploadID) + list = listParts(hc, bktName, objName, uploadInfo.UploadID, "0", http.StatusOK) require.Len(t, list.Parts, 2) require.Equal(t, etag1, list.Parts[0].ETag) require.Equal(t, etag2, list.Parts[1].ETag) @@ -219,6 +223,36 @@ func TestMultipartUploadSize(t *testing.T) { }) } +func TestListParts(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName, objName := "bucket-for-test-list-parts", "object-multipart" + _ = createTestBucket(hc, bktName) + partSize := 5 * 1024 * 1024 + + uploadInfo := createMultipartUpload(hc, bktName, objName, map[string]string{}) + etag1, _ := uploadPart(hc, bktName, objName, uploadInfo.UploadID, 1, partSize) + etag2, _ := uploadPart(hc, bktName, objName, uploadInfo.UploadID, 2, partSize) + + list := listParts(hc, bktName, objName, uploadInfo.UploadID, "0", http.StatusOK) + require.Len(t, list.Parts, 2) + require.Equal(t, etag1, list.Parts[0].ETag) + require.Equal(t, etag2, list.Parts[1].ETag) + + list = listParts(hc, bktName, objName, uploadInfo.UploadID, "1", http.StatusOK) + require.Len(t, list.Parts, 1) + require.Equal(t, etag2, list.Parts[0].ETag) + + list = listParts(hc, bktName, objName, uploadInfo.UploadID, "2", http.StatusOK) + require.Len(t, list.Parts, 0) + + list = listParts(hc, bktName, objName, uploadInfo.UploadID, "7", http.StatusOK) + require.Len(t, list.Parts, 0) + + list = listParts(hc, bktName, objName, uploadInfo.UploadID, "-1", http.StatusInternalServerError) + require.Len(t, list.Parts, 0) +} + 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) } @@ -267,13 +301,14 @@ func listMultipartUploadsBase(hc *handlerContext, bktName, prefix, delimiter, up return listPartsResponse } -func listParts(hc *handlerContext, bktName, objName string, uploadID string) *ListPartsResponse { - return listPartsBase(hc, bktName, objName, false, uploadID) +func listParts(hc *handlerContext, bktName, objName string, uploadID, partNumberMarker string, status int) *ListPartsResponse { + return listPartsBase(hc, bktName, objName, false, uploadID, partNumberMarker, status) } -func listPartsBase(hc *handlerContext, bktName, objName string, encrypted bool, uploadID string) *ListPartsResponse { +func listPartsBase(hc *handlerContext, bktName, objName string, encrypted bool, uploadID, partNumberMarker string, status int) *ListPartsResponse { query := make(url.Values) query.Set(uploadIDQuery, uploadID) + query.Set(partNumberMarkerQuery, partNumberMarker) w, r := prepareTestRequestWithQuery(hc, bktName, objName, query, nil) if encrypted { @@ -282,7 +317,7 @@ func listPartsBase(hc *handlerContext, bktName, objName string, encrypted bool, hc.Handler().ListPartsHandler(w, r) listPartsResponse := &ListPartsResponse{} - readResponse(hc.t, w, http.StatusOK, listPartsResponse) + readResponse(hc.t, w, status, listPartsResponse) return listPartsResponse } diff --git a/api/layer/multipart_upload.go b/api/layer/multipart_upload.go index 73dae82c..c48f5387 100644 --- a/api/layer/multipart_upload.go +++ b/api/layer/multipart_upload.go @@ -548,6 +548,10 @@ func (n *layer) ListParts(ctx context.Context, p *ListPartsParams) (*ListPartsIn return parts[i].PartNumber < parts[j].PartNumber }) + if p.PartNumberMarker >= parts[len(parts)-1].PartNumber { + res.Parts = make([]*Part, 0) + return &res, nil + } if p.PartNumberMarker != 0 { for i, part := range parts { if part.PartNumber > p.PartNumberMarker {