diff --git a/weed/s3api/filer_multipart.go b/weed/s3api/filer_multipart.go index 964400f6b..f992092da 100644 --- a/weed/s3api/filer_multipart.go +++ b/weed/s3api/filer_multipart.go @@ -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_.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 } diff --git a/weed/s3api/s3api_list_parts_no_such_upload_test.go b/weed/s3api/s3api_list_parts_no_such_upload_test.go index 591e7d653..c894571bb 100644 --- a/weed/s3api/s3api_list_parts_no_such_upload_test.go +++ b/weed/s3api/s3api_list_parts_no_such_upload_test.go @@ -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_.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 +}