From 08d5acf9637206ee72e748b3b8761147ff876b8e 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 75e8f7c..3d49051 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 f163875..7ec8863 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 0b5eb90..4bc4257 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 6038dad..9ea8d44 100644 --- 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"