From daa25ed6bc5bb7a3d8fb0450b8a945231b8b2ae0 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 2 Oct 2026 16:40:16 +0200 Subject: [PATCH] fix: accept upload-id-marker together with key-marker in ListMultipartUploads The posix and azure backends dropped every upload with a key equal to the key-marker before handing the list to the lister, which then looked for the upload-id-marker under the next key. An upload-id-marker that belongs to the key-marker could never be found, so the request failed with "Invalid uploadId marker", including when a client paged with the NextKeyMarker and NextUploadIdMarker the gateway returned. Keep the uploads of the key-marker itself when an upload-id-marker is set, so the listing continues right after the given upload, as S3 does. Enable the REST test that covers it. Fixes #2149 --- backend/azure/azure.go | 2 +- backend/mp-lister.go | 11 +++--- backend/posix/posix.go | 5 +-- tests/integration/ListMultipartUploads.go | 43 +++++++++++++++++++++++ tests/integration/group-tests.go | 2 ++ tests/test_rest_list_multipart_uploads.sh | 3 -- 6 files changed, 55 insertions(+), 11 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index df9516e8..4f8ec685 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -1760,7 +1760,7 @@ func (az *Azure) ListMultipartUploads(ctx context.Context, input *s3.ListMultipa if !ok { continue } - if keyMarker != "" && *key <= keyMarker { + if keyMarker != "" && (*key < keyMarker || (*key == keyMarker && uploadIDMarker == "")) { continue } if prefix != "" && !strings.HasPrefix(*key, prefix) { diff --git a/backend/mp-lister.go b/backend/mp-lister.go index 75e8f7c7..f5e0b8a6 100644 --- a/backend/mp-lister.go +++ b/backend/mp-lister.go @@ -42,7 +42,8 @@ func ListMultipartUploads(uploads []s3response.Upload, prefix, delimiter, keyMar // Assumptions about input Uploads: // - Sorted by (Key asc, Initiated asc) // - Filtered by Prefix -// - Filtered to start strictly after key-marker when key-marker was provided. +// - Filtered to start strictly after key-marker when key-marker was provided, +// or at key-marker when upload-id-marker was provided as well. type MultipartUploadLister struct { Uploads []s3response.Upload Prefix string @@ -148,13 +149,13 @@ func (l *MultipartUploadLister) Run() (*ListMultipartUploadsPage, error) { } // findUploadIdMarkerIndex finds the index of given uploadId marker in uploads -// uploadIDMarker must match an upload-id among uploads with the first key after KeyMarker. -// Since caller filtered to Key > KeyMarker and the list is sorted by key/time, -// the first key after KeyMarker is Uploads[0].Key (if any). +// uploadIDMarker must match an upload-id among uploads with the first key at or after KeyMarker. +// Since caller filtered to Key >= KeyMarker and the list is sorted by key/time, +// the first key at or after KeyMarker is Uploads[0].Key (if any). // -1 is returned if no uploadId is found func (l *MultipartUploadLister) findUploadIdMarkerIndex(uploadIDMarker string) int { if len(l.Uploads) == 0 { - // key-marker provided but nothing after it => upload-id-marker can never be valid. + // key-marker provided but nothing at or after it => upload-id-marker can never be valid. return -1 } firstKey := l.Uploads[0].Key diff --git a/backend/posix/posix.go b/backend/posix/posix.go index b30a63a6..f9cdc818 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -3472,8 +3472,9 @@ func (p *Posix) ListMultipartUploads(ctx context.Context, mpu *s3.ListMultipartU if prefix != "" && !strings.HasPrefix(objectName, prefix) { continue } - // filter by keyMarker - if keyMarker != "" && objectName <= keyMarker { + // filter by keyMarker, keeping the uploads of the key-marker + // itself when an upload-id-marker is set + if keyMarker != "" && (objectName < keyMarker || (objectName == keyMarker && uploadIDMarker == "")) { continue } diff --git a/tests/integration/ListMultipartUploads.go b/tests/integration/ListMultipartUploads.go index f163875e..e8bb4f41 100644 --- a/tests/integration/ListMultipartUploads.go +++ b/tests/integration/ListMultipartUploads.go @@ -346,6 +346,49 @@ func ListMultipartUploads_keyMarker_not_from_list(s *S3Conf) error { }) } +func ListMultipartUploads_upload_id_marker_with_key_marker(s *S3Conf) error { + testName := "ListMultipartUploads_upload_id_marker_with_key_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + for _, key := range []string{"bar", "baz", "baz", "foo"} { + _, err := createMp(s3client, bucket, key) + if err != nil { + return err + } + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + uploads := out.Uploads + + // key-marker and upload-id-marker of any listed upload + // should continue the listing right after that upload + for i, up := range uploads { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{ + Bucket: &bucket, + KeyMarker: up.Key, + UploadIdMarker: up.UploadId, + }) + cancel() + if err != nil { + return err + } + + if !compareMultipartUploads(uploads[i+1:], out.Uploads) { + return fmt.Errorf("expected the mp list to be %v, instead got %v", uploads[i+1:], out.Uploads) + } + } + + return nil + }) +} + func ListMultipartUploads_delimiter_truncated(s *S3Conf) error { testName := "ListMultipartUploads_delimiter_truncated" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 0b5eb904..7923968e 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -546,6 +546,7 @@ func TestListMultipartUploads(ts *TestState) { ts.Run(ListMultipartUploads_ignore_upload_id_marker) ts.Run(ListMultipartUploads_invalid_uploadId_marker) ts.Run(ListMultipartUploads_keyMarker_not_from_list) + ts.Run(ListMultipartUploads_upload_id_marker_with_key_marker) ts.Run(ListMultipartUploads_delimiter_truncated) ts.Run(ListMultipartUploads_prefix) ts.Run(ListMultipartUploads_both_delimiter_and_prefix) @@ -3254,6 +3255,7 @@ func GetIntTests() IntTests { "ListMultipartUploads_ignore_upload_id_marker": ListMultipartUploads_ignore_upload_id_marker, "ListMultipartUploads_invalid_uploadId_marker": ListMultipartUploads_invalid_uploadId_marker, "ListMultipartUploads_keyMarker_not_from_list": ListMultipartUploads_keyMarker_not_from_list, + "ListMultipartUploads_upload_id_marker_with_key_marker": ListMultipartUploads_upload_id_marker_with_key_marker, "ListMultipartUploads_delimiter_truncated": ListMultipartUploads_delimiter_truncated, "ListMultipartUploads_prefix": ListMultipartUploads_prefix, "ListMultipartUploads_both_delimiter_and_prefix": ListMultipartUploads_both_delimiter_and_prefix, diff --git a/tests/test_rest_list_multipart_uploads.sh b/tests/test_rest_list_multipart_uploads.sh index 6038dad1..08020b0c 100755 --- a/tests/test_rest_list_multipart_uploads.sh +++ b/tests/test_rest_list_multipart_uploads.sh @@ -81,9 +81,6 @@ source ./tests/setup.sh # tags: curl,ListMultipartUploads,multipart,uploadId,key-marker @test "REST - ListMultipartUploads - uploadId and key combo work" { - if [ "$DIRECT" != "true" ]; then - skip "https://github.com/versity/versitygw/issues/2149" - fi run setup_bucket_and_files_v3 "$BUCKET_ONE_NAME" 2 assert_success read -r bucket_name test_file_one test_file_two <<< "$output"