s3api: exclude marker part in listObjectParts pagination (#11463)

* s3api: exclude marker part in listObjectParts pagination

Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>

* s3api: guard listObjectParts marker boundary and enhance pagination test

Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>

* s3api: fold in review feedback from the parallel #11462 fix

Same core fix; this adds the explanatory comment, tightens the overflow
guard to math.MaxInt64, makes the fake filer sort entries like a real
listing, and adds the marker-exclusivity assertions alongside the
pagination walk.

Co-authored-by: yi111 <yi111@users.noreply.github.com>

---------

Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
Co-authored-by: yi111 <yi111@users.noreply.github.com>
This commit is contained in:
Mohd Quamar Tyagi
2026-09-26 11:58:23 +08:00
committed by GitHub
co-authored by yi111 Chris Lu
parent f31a026b2a
commit 975cec9228
2 changed files with 148 additions and 2 deletions
+19 -1
View File
@@ -1388,7 +1388,15 @@ func (s3a *S3ApiServer) listObjectParts(input *s3.ListPartsInput) (output *ListP
StorageClass: aws.String("STANDARD"),
}
entries, isLast, err := s3a.list(s3a.genUploadsFolder(*input.Bucket)+"/"+*input.UploadId, "", fmt.Sprintf("%04d%s", *input.PartNumberMarker, multipartExt), false, uint32(*input.MaxParts))
// part files are "%04d_<uuid>.part" (legacy "%04d.part"). '_' sorts after
// '.', so an exclusive start at "%04d.part" still returns the marker part
// and a page of max-parts=1 never advances. The next part number sorts
// past both name shapes of the marker part.
if *input.PartNumberMarker >= math.MaxInt64 {
output.IsTruncated = aws.Bool(false)
return output, s3err.ErrNone
}
entries, isLast, err := s3a.list(s3a.genUploadsFolder(*input.Bucket)+"/"+*input.UploadId, "", fmt.Sprintf("%04d", *input.PartNumberMarker+1), true, uint32(*input.MaxParts))
if err != nil {
// A store that reports the missing upload directory as not-found means
// the upload is gone (completed or aborted), not a store error.
@@ -1411,6 +1419,10 @@ func (s3a *S3ApiServer) listObjectParts(input *s3.ListPartsInput) (output *ListP
glog.Errorf("listObjectParts %s %s parse %s: %v", *input.Bucket, *input.UploadId, entry.Name, err)
continue
}
// the marker is exclusive even if a store ignores startFrom
if partNumber <= int(*input.PartNumberMarker) {
continue
}
partETag := getEtagFromEntry(entry)
part := &s3.Part{
@@ -1428,6 +1440,12 @@ func (s3a *S3ApiServer) listObjectParts(input *s3.ListPartsInput) (output *ListP
}
}
// a page that surfaced no parts has no next page to claim
if len(output.Part) == 0 {
output.IsTruncated = aws.Bool(false)
output.NextPartNumberMarker = nil
}
glog.V(2).Infof("listObjectParts: Returning %d parts for uploadId=%s", len(output.Part), *input.UploadId)
return
}
@@ -2,6 +2,8 @@ package s3api
import (
"context"
"math"
"sort"
"testing"
"github.com/aws/aws-sdk-go/aws"
@@ -42,10 +44,26 @@ func (f *fakePartsFiler) ListEntries(req *filer_pb.ListEntriesRequest, stream fi
}
// most stores list a missing directory as empty rather than erroring, which
// is exactly the behavior under test
for _, entry := range f.parts {
parts := append([]*filer_pb.Entry(nil), f.parts...)
sort.Slice(parts, func(i, j int) bool { return parts[i].Name < parts[j].Name })
var sent uint32
for _, entry := range parts {
name := entry.Name
if req.StartFromFileName != "" {
if name < req.StartFromFileName {
continue
}
if name == req.StartFromFileName && !req.InclusiveStartFrom {
continue
}
}
if err := stream.Send(&filer_pb.ListEntriesResponse{Entry: entry}); err != nil {
return err
}
sent++
if req.Limit > 0 && sent >= req.Limit {
return nil
}
}
return nil
}
@@ -150,3 +168,113 @@ func TestListPartsOpenUploadListsParts(t *testing.T) {
t.Fatalf("part[0] = %d/%d, want 1/5", *output.Part[0].PartNumber, *output.Part[0].Size)
}
}
// S3 part-number-marker is exclusive. Part objects are stored as
// NNNN_<uuid>.part, which sorts after the NNNN.part prefix, so a page of
// max-parts=1 must advance to the next part number instead of repeating it.
func TestListPartsPartNumberMarkerIsExclusive(t *testing.T) {
s3a := newListPartsServer(t, &fakePartsFiler{
uploadEntry: uploadRecordEntry("open-upload"),
parts: []*filer_pb.Entry{
partEntry("0001_11111111-1111-1111-1111-111111111111.part", 5),
partEntry("0002_22222222-2222-2222-2222-222222222222.part", 5),
partEntry("0003_33333333-3333-3333-3333-333333333333.part", 5),
},
})
input := listPartsInput("open-upload")
input.PartNumberMarker = aws.Int64(1)
input.MaxParts = aws.Int64(1)
output, code := s3a.listObjectParts(input)
if code != s3err.ErrNone {
t.Fatalf("code = %v, want ErrNone", code)
}
if got := partNumbers(output); len(got) != 1 || got[0] != 2 {
t.Fatalf("parts = %v, want [2]", got)
}
if output.IsTruncated == nil || !*output.IsTruncated {
t.Fatal("IsTruncated = false, want true")
}
if output.NextPartNumberMarker == nil || *output.NextPartNumberMarker != 2 {
t.Fatalf("NextPartNumberMarker = %v, want 2", output.NextPartNumberMarker)
}
input.MaxParts = aws.Int64(1000)
output, code = s3a.listObjectParts(input)
if code != s3err.ErrNone {
t.Fatalf("code = %v, want ErrNone", code)
}
if got := partNumbers(output); len(got) != 2 || got[0] != 2 || got[1] != 3 {
t.Fatalf("parts = %v, want [2 3]", got)
}
}
// A client paging one part at a time must walk marker=0 through every part and
// terminate: each page returns the next part and a NextPartNumberMarker that
// the following page starts strictly after.
func TestListPartsPaginationWithUUIDParts(t *testing.T) {
s3a := newListPartsServer(t, &fakePartsFiler{
uploadEntry: uploadRecordEntry("open-upload"),
parts: []*filer_pb.Entry{
partEntry("0001_c3a1e204-1111-1111-1111-111111111111.part", 5),
partEntry("0002_d4b2f315-2222-2222-2222-222222222222.part", 5),
partEntry("0003_e5c3a426-3333-3333-3333-333333333333.part", 5),
},
})
input := listPartsInput("open-upload")
input.MaxParts = aws.Int64(1)
for page, want := range []int64{1, 2, 3} {
output, code := s3a.listObjectParts(input)
if code != s3err.ErrNone {
t.Fatalf("page %d code = %v, want ErrNone", page+1, code)
}
if got := partNumbers(output); len(got) != 1 || got[0] != want {
t.Fatalf("page %d parts = %v, want [%d]", page+1, got, want)
}
if page < 2 {
if output.IsTruncated == nil || !*output.IsTruncated {
t.Fatalf("page %d IsTruncated = false, want true", page+1)
}
if output.NextPartNumberMarker == nil || *output.NextPartNumberMarker != want {
t.Fatalf("page %d NextPartNumberMarker = %v, want %d", page+1, output.NextPartNumberMarker, want)
}
input.PartNumberMarker = output.NextPartNumberMarker
} else {
if output.IsTruncated == nil || *output.IsTruncated {
t.Fatalf("page %d IsTruncated = true, want false", page+1)
}
if output.NextPartNumberMarker != nil {
t.Fatalf("page %d NextPartNumberMarker = %v, want nil", page+1, output.NextPartNumberMarker)
}
}
}
// a marker at the int64 ceiling cannot be incremented; answer an empty,
// untruncated page instead of wrapping the start name negative
input.PartNumberMarker = aws.Int64(math.MaxInt64)
output, code := s3a.listObjectParts(input)
if code != s3err.ErrNone {
t.Fatalf("max marker code = %v, want ErrNone", code)
}
if len(output.Part) != 0 {
t.Fatalf("max marker parts = %d, want 0", len(output.Part))
}
if output.IsTruncated == nil || *output.IsTruncated {
t.Fatal("max marker IsTruncated = true, want false")
}
}
func partNumbers(output *ListPartsResult) []int64 {
if output == nil {
return nil
}
nums := make([]int64, len(output.Part))
for i, part := range output.Part {
if part.PartNumber != nil {
nums[i] = *part.PartNumber
}
}
return nums
}