From 95a6da83b9498333a0a59b7d1a041431bd57a4af Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 2 Oct 2026 16:09:06 +0200 Subject: [PATCH] fix: report the last upload in ListMultipartUploads next markers on a single page NextKeyMarker and NextUploadIdMarker were only set for a truncated listing, so a ListMultipartUploads response that fit in one page returned them empty. S3 still sets them to the key and upload ID of the last upload in the response. Fill both markers from the last listed upload when the listing is not truncated, including the case where the page ends exactly at max-uploads. A page of common prefixes only keeps them empty, as before. Enable the REST test that covers it. Fixes #2144 --- backend/mp-lister.go | 14 ++++++- tests/integration/ListMultipartUploads.go | 51 +++++++++++++++++++++++ tests/integration/group-tests.go | 2 + tests/test_rest_list_multipart_uploads.sh | 3 -- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/backend/mp-lister.go b/backend/mp-lister.go index 75e8f7c7..3d49051c 100644 --- a/backend/mp-lister.go +++ b/backend/mp-lister.go @@ -126,8 +126,9 @@ func (l *MultipartUploadLister) Run() (*ListMultipartUploadsPage, error) { if out.IsTruncated { out.NextKeyMarker = lastKey out.NextUploadIDMarker = up.UploadID + return out, nil } - return out, nil + break } } continue @@ -139,11 +140,20 @@ func (l *MultipartUploadLister) Run() (*ListMultipartUploadsPage, error) { if out.IsTruncated { out.NextKeyMarker = lastKey out.NextUploadIDMarker = up.UploadID + return out, nil } - return out, nil + break } } + // a listing that is not truncated still reports its last upload + // in the next markers + if len(out.Uploads) != 0 { + last := out.Uploads[len(out.Uploads)-1] + out.NextKeyMarker = last.Key + out.NextUploadIDMarker = last.UploadID + } + return out, nil } diff --git a/tests/integration/ListMultipartUploads.go b/tests/integration/ListMultipartUploads.go index f163875e..7ec8863b 100644 --- a/tests/integration/ListMultipartUploads.go +++ b/tests/integration/ListMultipartUploads.go @@ -152,6 +152,57 @@ func ListMultipartUploads_max_uploads(s *S3Conf) error { }) } +func ListMultipartUploads_next_markers_not_truncated(s *S3Conf) error { + testName := "ListMultipartUploads_next_markers_not_truncated" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + uploads := []types.MultipartUpload{} + for _, key := range []string{"bar", "baz", "foo"} { + out, err := createMp(s3client, bucket, key) + if err != nil { + return err + } + uploads = append(uploads, types.MultipartUpload{ + UploadId: out.UploadId, + Key: out.Key, + StorageClass: types.StorageClassStandard, + }) + } + last := uploads[len(uploads)-1] + + // a single page still reports the last upload in the next markers, + // with the default max-uploads and with one equal to the upload count + for _, maxUploads := range []*int32{nil, getPtr(int32(len(uploads)))} { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{ + Bucket: &bucket, + MaxUploads: maxUploads, + }) + cancel() + if err != nil { + return err + } + + if out.IsTruncated == nil || *out.IsTruncated { + return fmt.Errorf("expected a non-truncated response") + } + if ok := compareMultipartUploads(out.Uploads, uploads); !ok { + return fmt.Errorf("expected multipart uploads to be %v, instead got %v", + uploads, out.Uploads) + } + if getString(out.NextKeyMarker) != getString(last.Key) { + return fmt.Errorf("expected next-key-marker to be %v, instead got %v", + getString(last.Key), getString(out.NextKeyMarker)) + } + if getString(out.NextUploadIdMarker) != getString(last.UploadId) { + return fmt.Errorf("expected next-upload-id-marker to be %v, instead got %v", + getString(last.UploadId), getString(out.NextUploadIdMarker)) + } + } + + return nil + }) +} + func ListMultipartUploads_exceeding_max_uploads(s *S3Conf) error { testName := "ListMultipartUploads_exceeding_max_uploads" 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..4bc42579 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -542,6 +542,7 @@ func TestListMultipartUploads(ts *TestState) { ts.Run(ListMultipartUploads_empty_result) ts.Run(ListMultipartUploads_invalid_max_uploads) ts.Run(ListMultipartUploads_max_uploads) + ts.Run(ListMultipartUploads_next_markers_not_truncated) ts.Run(ListMultipartUploads_exceeding_max_uploads) ts.Run(ListMultipartUploads_ignore_upload_id_marker) ts.Run(ListMultipartUploads_invalid_uploadId_marker) @@ -3250,6 +3251,7 @@ func GetIntTests() IntTests { "ListMultipartUploads_empty_result": ListMultipartUploads_empty_result, "ListMultipartUploads_invalid_max_uploads": ListMultipartUploads_invalid_max_uploads, "ListMultipartUploads_max_uploads": ListMultipartUploads_max_uploads, + "ListMultipartUploads_next_markers_not_truncated": ListMultipartUploads_next_markers_not_truncated, "ListMultipartUploads_exceeding_max_uploads": ListMultipartUploads_exceeding_max_uploads, "ListMultipartUploads_ignore_upload_id_marker": ListMultipartUploads_ignore_upload_id_marker, "ListMultipartUploads_invalid_uploadId_marker": ListMultipartUploads_invalid_uploadId_marker, diff --git a/tests/test_rest_list_multipart_uploads.sh b/tests/test_rest_list_multipart_uploads.sh index 6038dad1..9ea8d446 100755 --- a/tests/test_rest_list_multipart_uploads.sh +++ b/tests/test_rest_list_multipart_uploads.sh @@ -50,9 +50,6 @@ source ./tests/setup.sh # tags: curl,ListMultipartUploads,multipart,uploadId,next-key-marker,next-upload-id-marker @test "REST - ListMultipartUploads - NextKeyMarker and NextUploadIdMarker set to last item" { - if [ "$DIRECT" != "true" ]; then - skip "https://github.com/versity/versitygw/issues/2144" - fi run setup_bucket_and_files_v3 "$BUCKET_ONE_NAME" 2 assert_success read -r bucket_name test_file_one test_file_two <<< "$output"