s3: a list marker that sorts before the prefix excludes nothing (#11322)

* s3: a list marker that sorts before the prefix excludes nothing

ListObjects `marker` and ListObjectsV2 `start-after` are a plain key
cutoff: list the keys that sort after it. A marker that sorts before the
prefix and is not under it therefore excludes no key carrying the prefix,
and the listing must equal the one with no marker at all.

normalizePrefixMarker treated every marker that does not start with the
prefix as "something wrong" and the listing came back empty. Clients send
this shape routinely: docker/distribution's S3 storage driver walks
prefix "<root>/<path>/" with start-after "<root>" (its rootdirectory), so
on SeaweedFS a registry walk saw an empty bucket. zot read that as "no
repositories": /v2/_catalog was empty, GC/scrub/retention never saw a
repo, and on restart its storage parse deleted every repository's
metadata as "no longer in storage".

listFilerEntries now lists as if no marker were given when the marker
sorts before the prefix; the response still echoes the marker the client
sent. A marker that sorts after the prefix's subtree is left alone: it may
legitimately sit inside a partial-name prefix's match set, which
normalizePrefixMarker already handles, and otherwise correctly lists
nothing.

Reproduce on 4.44 and 4.47:

  curl -s "$S/zot?list-type=2&prefix=zot/zot/&start-after=zot/zot/"  # all keys
  curl -s "$S/zot?list-type=2&prefix=zot/zot/&start-after=zot"       # KeyCount 0
  curl -s "$S/zot?list-type=2&prefix=zot/zot/&start-after=a"         # KeyCount 0

* s3: keep the prefix's own key excluded by a marker that names it

Fold the before-prefix marker rule into normalizePrefixMarker, which now also
derives prefixEndsOnDelimiter from the effective marker instead of each cursor
rebuilding the expression.

A marker equal to the prefix is no longer trimmed to a subtree cutoff:
start-after "a/b/" with prefix "a/b/" excludes only the "a/b/" key, so the walk
starts inside that directory and its children still list.

Adds a listing-level test that walks the whole path for both start-after shapes
a registry sends, and covers the new normalization cases.

* s3: leading slashes do not hide a marker that names the prefix

* s3: echo the V1 marker the client sent, not the walk's cutoff

* s3: filter only the walk's cutoff from the V1 page, not the echoed marker

* s3: skip the key an exclusive marker names as it streams

---------

Co-authored-by: Zuse <be9c90a8-c104-4be2-b7a4-9f92eb833ac8@forge.local>
Co-authored-by: Chris Lu <chris.lu@gmail.com>

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
 }

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 "<root>/<path>/" with start-after "<root>".
+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)
+		})
+	}
+}

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)
 		})
 	}
 }