diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index f06fedebd..4b0e9cce5 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -109,12 +109,14 @@ func (s3a *S3ApiServer) ListObjectsV2Handler(w http.ResponseWriter, r *http.Requ } // Adjust marker if it ends with delimiter to skip all entries with that prefix - marker = adjustMarkerForDelimiter(marker, delimiter) + requestMarker := marker + marker = adjustMarkerForDelimiter(marker, originalPrefix, delimiter) response, err := s3a.listFilerEntries(r.Context(), listObjectsRequest{ bucket: bucket, prefix: originalPrefix, marker: marker, + requestMarker: requestMarker, delimiter: delimiter, maxKeys: maxKeys, encodingTypeUrl: encodingTypeUrl, @@ -181,12 +183,14 @@ func (s3a *S3ApiServer) ListObjectsV1Handler(w http.ResponseWriter, r *http.Requ } // Adjust marker if it ends with delimiter to skip all entries with that prefix - marker = adjustMarkerForDelimiter(marker, delimiter) + requestMarker := marker + marker = adjustMarkerForDelimiter(marker, originalPrefix, delimiter) response, err := s3a.listFilerEntries(r.Context(), listObjectsRequest{ bucket: bucket, prefix: originalPrefix, marker: marker, + requestMarker: requestMarker, delimiter: delimiter, maxKeys: uint16(maxKeys), encodingTypeUrl: encodingTypeUrl, @@ -251,9 +255,13 @@ func sanitizeV1MarkerEcho(response *ListBucketResult, marker string, encodingTyp } type listObjectsRequest struct { - bucket string - prefix string - marker string + bucket string + prefix string + marker string + // requestMarker is the marker as the client sent it, before a marker ending on the + // delimiter was trimmed to the walk's cutoff. The response echoes it, and no key it + // names is listed. + requestMarker string delimiter string maxKeys uint16 encodingTypeUrl bool @@ -264,8 +272,13 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq bucket, originalPrefix, originalMarker := req.bucket, req.prefix, req.marker maxKeys, delimiter := req.maxKeys, req.delimiter encodingTypeUrl, fetchOwner := req.encodingTypeUrl, req.fetchOwner + requestMarker := req.requestMarker + if requestMarker == "" { + requestMarker = originalMarker + } + excludedKey := excludedMarkerKey(requestMarker, originalMarker) // convert full path prefix into directory name and prefix for entry name - requestDir, prefix, marker := normalizePrefixMarker(originalPrefix, originalMarker) + requestDir, prefix, marker, prefixEndsOnDelimiter := normalizePrefixMarker(originalPrefix, originalMarker) bucketPrefix := s3a.bucketPrefix(bucket) reqDir := bucketPrefix[:len(bucketPrefix)-1] if requestDir != "" { @@ -278,7 +291,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq var nextMarker string cursor := &ListingCursor{ maxKeys: maxKeys, - prefixEndsOnDelimiter: strings.HasSuffix(originalPrefix, "/") && len(originalMarker) == 0, + prefixEndsOnDelimiter: prefixEndsOnDelimiter, } // Special case: when maxKeys = 0, return empty results immediately with IsTruncated=false @@ -286,7 +299,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq response = ListBucketResult{ Name: bucket, Prefix: originalPrefix, - Marker: originalMarker, + Marker: requestMarker, NextMarker: "", MaxKeys: int(maxKeys), Delimiter: delimiter, @@ -314,7 +327,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq marker = alignedMarker *cursor = ListingCursor{ maxKeys: maxKeys, - prefixEndsOnDelimiter: strings.HasSuffix(originalPrefix, "/") && len(originalMarker) == 0, + prefixEndsOnDelimiter: prefixEndsOnDelimiter, } var lastEntryWasCommonPrefix bool @@ -524,6 +537,9 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq nextMarker, doErr = s3a.doListFilerEntries(ctx, client, listDirectoryRequest{dir: reqDir, prefix: prefix, marker: marker, delimiter: delimiter, bucket: bucket}, cursor, func(dir string, entry *filer_pb.Entry) { empty = false prunePendingNulls(dir, entry.Name) + if excludedKey != "" && !entry.IsDirectory && fmt.Sprintf("%s/%s", dir, entry.Name)[len(bucketPrefix):] == excludedKey { + return + } dirName, entryName, _ := entryUrlEncode(dir, entry.Name, encodingTypeUrl) if entry.IsDirectory { if originalPrefix != "" { @@ -726,7 +742,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq response = ListBucketResult{ Name: bucket, Prefix: originalPrefix, - Marker: originalMarker, + Marker: requestMarker, NextMarker: nextMarker, MaxKeys: int(maxKeys), Delimiter: delimiter, @@ -772,9 +788,42 @@ type ListingCursor struct { resolvePendingNulls func() error } +// excludedMarkerKey returns the key an exclusive marker names that the walk's cutoff no +// longer excludes, because a marker ending on the delimiter is trimmed to that cutoff. +// The key is skipped as it streams, rather than spending a slot of the page and being +// dropped from the answer afterwards, which would turn a truncated page into a final one. +func excludedMarkerKey(requestMarker, marker string) string { + if requestMarker == marker { + return "" + } + return strings.TrimLeft(requestMarker, "/") +} + +// markerSortsBeforePrefix reports whether marker is a cutoff that excludes no key +// under prefix: the marker sorts before the prefix and is not under it, so every key +// carrying the prefix already sorts after it. A marker that sorts after the prefix is +// left alone: it may sit inside a partial name prefix's match set ("parent" also +// matches "parentDir/…"), which normalizePrefixMarker handles. +func markerSortsBeforePrefix(prefix, marker string) bool { + prefix = strings.TrimLeft(prefix, "/") + marker = strings.TrimLeft(marker, "/") + if marker == "" || prefix == "" { + return false + } + return !strings.HasPrefix(marker, prefix) && marker < prefix +} + // the prefix and marker may be in different directories -// normalizePrefixMarker ensures the prefix and marker both starts from the same directory -func normalizePrefixMarker(prefix, marker string) (alignedDir, alignedPrefix, alignedMarker string) { +// normalizePrefixMarker ensures the prefix and marker both starts from the same directory. +// prefixEndsOnDelimiter tells the walk that the prefix names one directory, whose own key +// is in scope. +func normalizePrefixMarker(prefix, marker string) (alignedDir, alignedPrefix, alignedMarker string, prefixEndsOnDelimiter bool) { + // A marker that excludes no key under the prefix is dropped, so the listing is the + // one with no marker at all. The response still echoes the marker the client sent. + if markerSortsBeforePrefix(prefix, marker) { + marker = "" + } + prefixEndsOnDelimiter = strings.HasSuffix(prefix, "/") && len(marker) == 0 // alignedDir should not end with "/" // alignedDir, alignedPrefix, alignedMarker should only have "/" in middle if len(marker) == 0 { @@ -784,7 +833,7 @@ func normalizePrefixMarker(prefix, marker string) (alignedDir, alignedPrefix, al } marker = strings.TrimLeft(marker, "/") if prefix == "" { - return "", "", marker + return "", "", marker, prefixEndsOnDelimiter } if marker == "" { alignedDir, alignedPrefix = toDirAndName(prefix) @@ -792,7 +841,7 @@ func normalizePrefixMarker(prefix, marker string) (alignedDir, alignedPrefix, al } if !strings.HasPrefix(marker, prefix) { // something wrong - return "", prefix, marker + return "", prefix, marker, prefixEndsOnDelimiter } // Resolve the listing dir from the prefix, not the marker: a partial name prefix like // "data/a" also matches siblings such as "data/ab/", which narrowing to the marker's @@ -1311,18 +1360,23 @@ func compareWithDelimiter(a, b, delimiter string) bool { // but still finds any "bop" or later entries. We add a high ASCII character rather than incrementing // the last character to avoid skipping potential directory entries. // This is essential for correct S3 list operations with delimiters and CommonPrefixes. -func adjustMarkerForDelimiter(marker, delimiter string) string { +// A marker equal to the prefix names no subtree to skip: it excludes only the prefix's own +// key. Leading slashes are insignificant here, as they are to normalizePrefixMarker. +func adjustMarkerForDelimiter(marker, prefix, delimiter string) string { if delimiter == "" || !strings.HasSuffix(marker, delimiter) { return marker } + if strings.TrimLeft(marker, "/") == strings.TrimLeft(prefix, "/") { + return marker + } // Remove the trailing delimiter // This ensures we skip all entries under the prefix but don't skip // potential directory entries that start with a similar prefix - prefix := strings.TrimSuffix(marker, delimiter) - if len(prefix) == 0 { + trimmed := strings.TrimSuffix(marker, delimiter) + if len(trimmed) == 0 { return marker } - return prefix + return trimmed } diff --git a/weed/s3api/s3api_object_handlers_list_marker_before_prefix_test.go b/weed/s3api/s3api_object_handlers_list_marker_before_prefix_test.go new file mode 100644 index 000000000..349254234 --- /dev/null +++ b/weed/s3api/s3api_object_handlers_list_marker_before_prefix_test.go @@ -0,0 +1,94 @@ +package s3api + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/stretchr/testify/assert" +) + +// A marker that sorts before the prefix excludes nothing under the prefix, so the +// listing must be the one with no marker. docker/distribution's S3 driver sends +// exactly this shape: prefix "//" with start-after "". +func Test_markerSortsBeforePrefix(t *testing.T) { + tests := []struct { + name string + prefix string + marker string + want bool + }{ + {"marker is the root directory the prefix walks under", "docker/registry/", "docker", true}, + {"marker is the prefix directory without its slash", "docker/registry/", "docker/registry", true}, + {"marker is an unrelated earlier key", "docker/registry/", "a", true}, + {"marker is an earlier sibling", "docker/registry/", "docker/regisr", true}, + {"leading slashes are ignored on both sides", "/docker/registry/", "/docker", true}, + {"empty marker is not a cutoff", "docker/registry/", "", false}, + {"empty prefix: every key is in scope, marker stands", "", "docker", false}, + {"marker under the prefix resumes inside it", "docker/registry/", "docker/registry/v2/link", false}, + {"marker equal to the prefix stands", "docker/registry/", "docker/registry/", false}, + {"marker after the prefix's subtree stands", "docker/registry/", "docker/registryx", false}, + {"partial name prefix: a later match-set marker stands", "parent", "parentDir/data/0e", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, markerSortsBeforePrefix(tt.prefix, tt.marker), "markerSortsBeforePrefix(%q, %q)", tt.prefix, tt.marker) + }) + } +} + +// A marker ending on the delimiter is trimmed to a shorter cutoff for the walk, which no +// longer excludes the key the client named, so that key is skipped as it streams. +func Test_excludedMarkerKey(t *testing.T) { + tests := []struct { + name string + requestMarker string + marker string + want string + }{ + {"marker trimmed to a shorter cutoff", "docker/", "docker", "docker/"}, + {"leading slashes are dropped, as the keys carry none", "/docker/", "docker", "docker/"}, + {"untrimmed marker: the walk already excludes it", "docker", "docker", ""}, + {"no marker", "", "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, excludedMarkerKey(tt.requestMarker, tt.marker), "excludedMarkerKey(%q, %q)", tt.requestMarker, tt.marker) + }) + } +} + +// TestListWithMarkerBeforePrefix walks the whole listing path the way listFilerEntries +// does, for the two start-after values a registry sends against the same prefix. +func TestListWithMarkerBeforePrefix(t *testing.T) { + client := &testFilerClient{ + entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/registry/docker/registry/v2": {newDir("repositories")}, + "/buckets/registry/docker/registry/v2/repositories": {newDir("app")}, + "/buckets/registry/docker/registry/v2/repositories/app": { + {Name: "link", Attributes: &filer_pb.FuseAttributes{}}, + }, + }, + } + prefix := "docker/registry/v2/repositories/" + + for _, tt := range []struct { + name string + marker string + }{ + {"start-after is the root directory the walk starts from", "docker"}, + {"start-after is the prefix itself", prefix}, + {"start-after is the prefix with a leading slash", "/" + prefix}, + } { + t.Run(tt.name, func(t *testing.T) { + marker := adjustMarkerForDelimiter(tt.marker, prefix, "/") + requestDir, entryPrefix, entryMarker, prefixEndsOnDelimiter := normalizePrefixMarker(prefix, marker) + seen := listedNames(t, client, listDirectoryRequest{ + dir: "/buckets/registry/" + requestDir, + prefix: entryPrefix, + marker: entryMarker, + bucket: "registry", + }, &ListingCursor{maxKeys: 1000, prefixEndsOnDelimiter: prefixEndsOnDelimiter}) + assert.Equal(t, []string{"link"}, seen) + }) + } +} diff --git a/weed/s3api/s3api_object_handlers_list_test.go b/weed/s3api/s3api_object_handlers_list_test.go index 9cffaee85..1d87c6b8a 100644 --- a/weed/s3api/s3api_object_handlers_list_test.go +++ b/weed/s3api/s3api_object_handlers_list_test.go @@ -166,11 +166,12 @@ func Test_normalizePrefixMarker(t *testing.T) { marker string } tests := []struct { - name string - args args - wantAlignedDir string - wantAlignedPrefix string - wantAlignedMarker string + name string + args args + wantAlignedDir string + wantAlignedPrefix string + wantAlignedMarker string + wantPrefixEndsOnDelimiter bool }{ {"bucket root listing with delimiter", args{"/", @@ -178,6 +179,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "", "", "", + true, }, {"prefix is a directory", args{"/parentDir/data/", @@ -185,6 +187,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "parentDir", "data", "", + true, }, {"normal case", args{"/parentDir/data/0", @@ -192,6 +195,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "parentDir/data", "0", "0e/0e149049a2137b0cc12e", + false, }, {"empty prefix", args{"", @@ -199,6 +203,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "", "", "parentDir/data/0e/0e149049a2137b0cc12e", + false, }, {"empty directory", args{"parent", @@ -206,6 +211,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "", "parent", "parentDir/data/0e/0e149049a2137b0cc12e", + false, }, {"partial name prefix, marker resumes inside a matching subdirectory", args{"data/a", @@ -213,6 +219,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "data", "a", "a/1", + false, }, {"partial name prefix, marker resumes inside a matching sibling directory", args{"data/a", @@ -220,6 +227,7 @@ func Test_normalizePrefixMarker(t *testing.T) { "data", "a", "ab/1", + false, }, {"top-level partial name prefix, marker resumes inside a matching subdirectory", args{"a", @@ -227,14 +235,32 @@ func Test_normalizePrefixMarker(t *testing.T) { "", "a", "a/1", + false, + }, + {"marker sorts before the prefix, so it excludes nothing under it", + args{"parentDir/data/", + "parentDir"}, + "parentDir", + "data", + "", + true, + }, + {"marker is the prefix directory, whose own key it excludes", + args{"parentDir/data/", + "parentDir/data/"}, + "parentDir/data", + "", + "", + false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotAlignedDir, gotAlignedPrefix, gotAlignedMarker := normalizePrefixMarker(tt.args.prefix, tt.args.marker) + gotAlignedDir, gotAlignedPrefix, gotAlignedMarker, gotPrefixEndsOnDelimiter := normalizePrefixMarker(tt.args.prefix, tt.args.marker) assert.Equalf(t, tt.wantAlignedDir, gotAlignedDir, "normalizePrefixMarker(%v, %v)", tt.args.prefix, tt.args.marker) assert.Equalf(t, tt.wantAlignedPrefix, gotAlignedPrefix, "normalizePrefixMarker(%v, %v)", tt.args.prefix, tt.args.marker) assert.Equalf(t, tt.wantAlignedMarker, gotAlignedMarker, "normalizePrefixMarker(%v, %v)", tt.args.prefix, tt.args.marker) + assert.Equalf(t, tt.wantPrefixEndsOnDelimiter, gotPrefixEndsOnDelimiter, "normalizePrefixMarker(%v, %v)", tt.args.prefix, tt.args.marker) }) } }