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
This commit is contained in:
1 parent
f81c6a5acb
commit
cbe870a618
6 files changed
+55
-11
No files matched your search
@@ -1760,7 +1760,7 @@ func (az *Azure) ListMultipartUploads(ctx context.Context, input *s3.ListMultipa
|
|||||||
if !ok {
|
if !ok {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if keyMarker != "" && *key <= keyMarker {
|
if keyMarker != "" && (*key < keyMarker || (*key == keyMarker && uploadIDMarker == "")) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if prefix != "" && !strings.HasPrefix(*key, prefix) {
|
if prefix != "" && !strings.HasPrefix(*key, prefix) {
|
||||||
|
|||||||
@@ -42,7 +42,8 @@ func ListMultipartUploads(uploads []s3response.Upload, prefix, delimiter, keyMar
|
|||||||
// Assumptions about input Uploads:
|
// Assumptions about input Uploads:
|
||||||
// - Sorted by (Key asc, Initiated asc)
|
// - Sorted by (Key asc, Initiated asc)
|
||||||
// - Filtered by Prefix
|
// - 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 {
|
type MultipartUploadLister struct {
|
||||||
Uploads []s3response.Upload
|
Uploads []s3response.Upload
|
||||||
Prefix string
|
Prefix string
|
||||||
@@ -148,13 +149,13 @@ func (l *MultipartUploadLister) Run() (*ListMultipartUploadsPage, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// findUploadIdMarkerIndex finds the index of given uploadId marker in uploads
|
// findUploadIdMarkerIndex finds the index of given uploadId marker in uploads
|
||||||
// uploadIDMarker must match an upload-id among uploads with the first key after KeyMarker.
|
// 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,
|
// 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).
|
// the first key at or after KeyMarker is Uploads[0].Key (if any).
|
||||||
// -1 is returned if no uploadId is found
|
// -1 is returned if no uploadId is found
|
||||||
func (l *MultipartUploadLister) findUploadIdMarkerIndex(uploadIDMarker string) int {
|
func (l *MultipartUploadLister) findUploadIdMarkerIndex(uploadIDMarker string) int {
|
||||||
if len(l.Uploads) == 0 {
|
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
|
return -1
|
||||||
}
|
}
|
||||||
firstKey := l.Uploads[0].Key
|
firstKey := l.Uploads[0].Key
|
||||||
|
|||||||
@@ -3472,8 +3472,9 @@ func (p *Posix) ListMultipartUploads(ctx context.Context, mpu *s3.ListMultipartU
|
|||||||
if prefix != "" && !strings.HasPrefix(objectName, prefix) {
|
if prefix != "" && !strings.HasPrefix(objectName, prefix) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// filter by keyMarker
|
// filter by keyMarker, keeping the uploads of the key-marker
|
||||||
if keyMarker != "" && objectName <= keyMarker {
|
// itself when an upload-id-marker is set
|
||||||
|
if keyMarker != "" && (objectName < keyMarker || (objectName == keyMarker && uploadIDMarker == "")) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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 {
|
func ListMultipartUploads_delimiter_truncated(s *S3Conf) error {
|
||||||
testName := "ListMultipartUploads_delimiter_truncated"
|
testName := "ListMultipartUploads_delimiter_truncated"
|
||||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||||
|
|||||||
@@ -546,6 +546,7 @@ func TestListMultipartUploads(ts *TestState) {
|
|||||||
ts.Run(ListMultipartUploads_ignore_upload_id_marker)
|
ts.Run(ListMultipartUploads_ignore_upload_id_marker)
|
||||||
ts.Run(ListMultipartUploads_invalid_uploadId_marker)
|
ts.Run(ListMultipartUploads_invalid_uploadId_marker)
|
||||||
ts.Run(ListMultipartUploads_keyMarker_not_from_list)
|
ts.Run(ListMultipartUploads_keyMarker_not_from_list)
|
||||||
|
ts.Run(ListMultipartUploads_upload_id_marker_with_key_marker)
|
||||||
ts.Run(ListMultipartUploads_delimiter_truncated)
|
ts.Run(ListMultipartUploads_delimiter_truncated)
|
||||||
ts.Run(ListMultipartUploads_prefix)
|
ts.Run(ListMultipartUploads_prefix)
|
||||||
ts.Run(ListMultipartUploads_both_delimiter_and_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_ignore_upload_id_marker": ListMultipartUploads_ignore_upload_id_marker,
|
||||||
"ListMultipartUploads_invalid_uploadId_marker": ListMultipartUploads_invalid_uploadId_marker,
|
"ListMultipartUploads_invalid_uploadId_marker": ListMultipartUploads_invalid_uploadId_marker,
|
||||||
"ListMultipartUploads_keyMarker_not_from_list": ListMultipartUploads_keyMarker_not_from_list,
|
"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_delimiter_truncated": ListMultipartUploads_delimiter_truncated,
|
||||||
"ListMultipartUploads_prefix": ListMultipartUploads_prefix,
|
"ListMultipartUploads_prefix": ListMultipartUploads_prefix,
|
||||||
"ListMultipartUploads_both_delimiter_and_prefix": ListMultipartUploads_both_delimiter_and_prefix,
|
"ListMultipartUploads_both_delimiter_and_prefix": ListMultipartUploads_both_delimiter_and_prefix,
|
||||||
|
|||||||
@@ -81,9 +81,6 @@ source ./tests/setup.sh
|
|||||||
|
|
||||||
# tags: curl,ListMultipartUploads,multipart,uploadId,key-marker
|
# tags: curl,ListMultipartUploads,multipart,uploadId,key-marker
|
||||||
@test "REST - ListMultipartUploads - uploadId and key combo work" {
|
@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
|
run setup_bucket_and_files_v3 "$BUCKET_ONE_NAME" 2
|
||||||
assert_success
|
assert_success
|
||||||
read -r bucket_name test_file_one test_file_two <<< "$output"
|
read -r bucket_name test_file_one test_file_two <<< "$output"
|
||||||
|
|||||||
Reference in new issue
Block a user