weed/s3api: avoid truncated marker rescan in ListObjects (#12540) (#12541)

* weed/s3api: avoid truncated marker rescan in ListObjects (#12540)

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

* fix(s3): validate marker directory candidates

Respect prefix filters and internal-directory exclusions when recovering marker candidates. Restrict lookups to direct children, propagate read failures other than not-found, and deduplicate directory marker echoes.

Add regression coverage and model promoted objects consistently in the XML-unsafe prefix fixture.

---------

Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>

weed/s3api/s3api_object_handlers_list.go

@@ -1066,18 +1066,11 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb.
 
 	// A directory's keys sort at "<name>/" among this level's children, not at
 	// the name itself: siblings like "a-c" and "a.txt" sort before "a/b" while
-	// "a0" sorts after. Start the window at the first child whose block can
-	// still hold keys past the marker — the shortest prefix of the marker
-	// followed by a byte no greater than "/" — and ask for it inclusively, so
-	// a marker-named directory's subtree is reached at all.
+	// "a0" sorts after. Any candidate directory whose prefix sorts before the marker
+	// but whose "<name>/" block extends past the marker is looked up and queued into
+	// pendingDirs below. The main window starts strictly after the marker.
 	startName := marker
-	for i := 0; i < len(marker); i++ {
-		if marker[i] <= '/' {
-			startName = marker[:i]
-			break
-		}
-	}
-	inclusiveStartFrom = startName != ""
+	inclusiveStartFrom = false
 
 	// now marker is also a direct child of dir
 	ctx, cancel := context.WithCancel(ctx)
@@ -1204,11 +1197,42 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb.
 		for i < len(pendingDirs) && pendingDirs[i].e.Name+"/" < blockKey {
 			i++
 		}
+		// A backend may echo a marker directory already queued by its lookup.
+		if i < len(pendingDirs) && pendingDirs[i].e.Name == pd.e.Name {
+			return
+		}
 		pendingDirs = append(pendingDirs, pendingDir{})
 		copy(pendingDirs[i+1:], pendingDirs[i:])
 		pendingDirs[i] = pd
 	}
 
+	// Look up candidate directory prefixes that could have their "<name>/" block
+	// sort past the marker or contain the marker, and queue them into pendingDirs.
+	// Only direct children belong here; recursion handles the rest of the marker.
+	if marker != "" {
+		markerName, _, _ := strings.Cut(marker, "/")
+		for i := 1; i <= len(markerName); i++ {
+			if i == len(markerName) || markerName[i] <= '/' {
+				candidate := markerName[:i]
+				blockKey := candidate + "/"
+				if strings.HasPrefix(candidate, prefix) && (blockKey > marker || strings.HasPrefix(marker, blockKey)) {
+					resp, lookupErr := filer_pb.LookupEntry(ctx, client, &filer_pb.LookupDirectoryEntryRequest{Directory: dir, Name: candidate})
+					if errors.Is(lookupErr, filer_pb.ErrNotFound) {
+						continue
+					}
+					if lookupErr != nil {
+						return "", fmt.Errorf("lookup marker candidate %s/%s: %w", dir, candidate, lookupErr)
+					}
+					entry := resp.Entry
+					if listWalkDescends(entry) {
+						matchedPrefixDir := cursor.prefixEndsOnDelimiter && entry.Name == prefix
+						queueDir(pendingDir{e: entry, matchedPrefixDir: matchedPrefixDir})
+					}
+				}
+			}
+		}
+	}
+
 	// Entries that emit nothing (empty directories, the .uploads folder, the marker
 	// echo) consume the request window without consuming maxKeys, so one window may
 	// end before maxKeys is satisfied. Keep requesting from the last received entry

weed/s3api/s3api_object_handlers_list_test.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"net/http/httptest"
 	"os"
+	"path"
 	"strings"
 	"testing"
 	"time"
@@ -14,7 +15,9 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
 	"github.com/stretchr/testify/assert"
 	grpc "google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 )
 
 type testListEntriesStream struct {
@@ -75,8 +78,10 @@ func (c *testFilerClient) ListEntries(ctx context.Context, in *filer_pb.ListEntr
 }
 
 func (c *testFilerClient) LookupDirectoryEntry(ctx context.Context, in *filer_pb.LookupDirectoryEntryRequest, opts ...grpc.CallOption) (*filer_pb.LookupDirectoryEntryResponse, error) {
-	for _, e := range c.entriesByDir[in.Directory] {
-		if e.Name == in.Name {
+	// The real filer resolves the joined path and returns the entry's basename.
+	dir, name := path.Split(path.Join(in.Directory, in.Name))
+	for _, e := range c.entriesByDir[strings.TrimSuffix(dir, "/")] {
+		if e.Name == name {
 			return &filer_pb.LookupDirectoryEntryResponse{Entry: e}, nil
 		}
 	}
@@ -127,6 +132,15 @@ func (c *markerEchoFilerClient) ListEntries(ctx context.Context, in *filer_pb.Li
 	return &testListEntriesStream{entries: filtered}, nil
 }
 
+func (c *markerEchoFilerClient) LookupDirectoryEntry(ctx context.Context, in *filer_pb.LookupDirectoryEntryRequest, opts ...grpc.CallOption) (*filer_pb.LookupDirectoryEntryResponse, error) {
+	for _, e := range c.entriesByDir[in.Directory] {
+		if e.Name == in.Name {
+			return &filer_pb.LookupDirectoryEntryResponse{Entry: e}, nil
+		}
+	}
+	return nil, filer_pb.ErrNotFound
+}
+
 func ensureEntryAttributes(entries []*filer_pb.Entry) {
 	for _, entry := range entries {
 		if entry == nil {
@@ -1478,3 +1492,258 @@ func TestDoListFilerEntries_DirKeyObjectBoundaryTruncates(t *testing.T) {
 	assert.Equal(t, []string{"/a/x"}, seen)
 	assert.False(t, cursor.isTruncated)
 }
+
+type requestRecordingFilerClient struct {
+	*testFilerClient
+	lastRequest *filer_pb.ListEntriesRequest
+}
+
+func (c *requestRecordingFilerClient) ListEntries(ctx context.Context, in *filer_pb.ListEntriesRequest, opts ...grpc.CallOption) (grpc.ServerStreamingClient[filer_pb.ListEntriesResponse], error) {
+	c.lastRequest = in
+	return c.testFilerClient.ListEntries(ctx, in, opts...)
+}
+
+func TestDoListFilerEntries_MarkerStartsDirectlyWithoutTruncation(t *testing.T) {
+	s3a := &S3ApiServer{}
+	file := func(name string) *filer_pb.Entry {
+		return &filer_pb.Entry{Name: name, Attributes: &filer_pb.FuseAttributes{}}
+	}
+	baseClient := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/b": {
+				file("2026-11-01-a"),
+				file("2026-11-01-b"),
+				file("2026-11-02-a"),
+				file("2026-11-02-b"),
+			},
+		},
+	}
+	client := &requestRecordingFilerClient{testFilerClient: baseClient}
+
+	cursor := &ListingCursor{maxKeys: 1000}
+	var seen []string
+	nextMarker, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{dir: "/buckets/b", marker: "2026-11-01-b", bucket: "b"}, cursor, func(dir string, entry *filer_pb.Entry) {
+		seen = append(seen, entry.Name)
+		cursor.maxKeys--
+	})
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"2026-11-02-a", "2026-11-02-b"}, seen)
+	assert.Equal(t, "2026-11-02-b", nextMarker)
+	assert.False(t, cursor.isTruncated)
+	assert.NotNil(t, client.lastRequest)
+	assert.Equal(t, "2026-11-01-b", client.lastRequest.StartFromFileName, "must start strictly from the marker rather than truncated prefix 2026")
+	assert.False(t, client.lastRequest.InclusiveStartFrom)
+}
+
+func TestDoListFilerEntries_CandidateDirectoryPrefixPastMarker(t *testing.T) {
+	s3a := &S3ApiServer{}
+	file := func(name string) *filer_pb.Entry {
+		return &filer_pb.Entry{Name: name, Attributes: &filer_pb.FuseAttributes{}}
+	}
+	dirObj := func(name string) *filer_pb.Entry {
+		return &filer_pb.Entry{Name: name, IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}}
+	}
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/b": {
+				dirObj("a"),
+				file("a-c"),
+				file("a0"),
+			},
+			"/buckets/b/a": {
+				file("b"),
+			},
+		},
+	}
+
+	cursor := &ListingCursor{maxKeys: 1000}
+	var seen []string
+	_, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{dir: "/buckets/b", marker: "a-c", bucket: "b"}, cursor, func(dir string, entry *filer_pb.Entry) {
+		seen = append(seen, strings.TrimPrefix(dir, "/buckets/b")+"/"+entry.Name)
+		cursor.maxKeys--
+	})
+	assert.NoError(t, err)
+	// Byte order: a-c < a/b < a0. Listing from marker "a-c" must find /a/b and /a0.
+	assert.Equal(t, []string{"/a/b", "/a0"}, seen)
+}
+
+func TestDoListFilerEntries_CandidateRespectsPrefix(t *testing.T) {
+	for _, delimiter := range []string{"", "/"} {
+		t.Run("delimiter="+delimiter, func(t *testing.T) {
+			client := &testFilerClient{entriesByDir: map[string][]*filer_pb.Entry{
+				"/buckets/b": {
+					{Name: "a", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+					{Name: "a-bc", Attributes: &filer_pb.FuseAttributes{}},
+					{Name: "a-bd", Attributes: &filer_pb.FuseAttributes{}},
+					{Name: "a-be", Attributes: &filer_pb.FuseAttributes{}},
+				},
+				"/buckets/b/a": {{Name: "b", Attributes: &filer_pb.FuseAttributes{}}},
+			}}
+			cursor := &ListingCursor{maxKeys: 10}
+			var seen []string
+			s3a := &S3ApiServer{}
+			_, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{
+				dir: "/buckets/b", prefix: "a-b", marker: "a-bc", delimiter: delimiter, bucket: "b",
+			}, cursor, func(dir string, entry *filer_pb.Entry) {
+				seen = append(seen, strings.TrimPrefix(dir, "/buckets/b")+"/"+entry.Name)
+				cursor.maxKeys--
+			})
+			assert.NoError(t, err)
+			assert.Equal(t, []string{"/a-bd", "/a-be"}, seen)
+			assert.Equal(t, uint16(8), cursor.maxKeys, "unrelated candidates must not consume quota")
+			assert.False(t, cursor.isTruncated)
+		})
+	}
+}
+
+func TestDoListFilerEntries_CandidateSkipsInternalDirectories(t *testing.T) {
+	for _, name := range []string{s3_constants.MultipartUploadsFolder, s3_constants.VersionsFolder, "foo" + s3_constants.VersionsFolder} {
+		for _, marker := range []string{name, name + "-cutoff", name + "/stored-version"} {
+			for _, delimiter := range []string{"", "/"} {
+				t.Run(marker+"/delimiter="+delimiter, func(t *testing.T) {
+					client := &testFilerClient{entriesByDir: map[string][]*filer_pb.Entry{
+						"/buckets/b": {
+							{Name: name, IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+							{Name: "z", Attributes: &filer_pb.FuseAttributes{}},
+						},
+						"/buckets/b/" + name: {
+							{Name: "stored-version", Attributes: &filer_pb.FuseAttributes{}},
+							{Name: "z-version", Attributes: &filer_pb.FuseAttributes{}},
+						},
+					}}
+					cursor := &ListingCursor{maxKeys: 1}
+					var seen []string
+					s3a := &S3ApiServer{}
+					_, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{
+						dir: "/buckets/b", marker: marker, delimiter: delimiter, bucket: "b",
+					}, cursor, func(dir string, entry *filer_pb.Entry) {
+						seen = append(seen, strings.TrimPrefix(dir, "/buckets/b")+"/"+entry.Name)
+						cursor.maxKeys--
+					})
+					assert.NoError(t, err)
+					assert.Equal(t, []string{"/z"}, seen)
+					assert.False(t, cursor.isTruncated)
+				})
+			}
+		}
+	}
+}
+
+func TestDoListFilerEntries_CandidateOnlyLooksUpDirectChildren(t *testing.T) {
+	client := &testFilerClient{entriesByDir: map[string][]*filer_pb.Entry{
+		"/buckets/b": {
+			{Name: "a", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+			{Name: "b", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+			{Name: "z", Attributes: &filer_pb.FuseAttributes{}},
+		},
+		"/buckets/b/a": {
+			{Name: "b", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+			{Name: "c", Attributes: &filer_pb.FuseAttributes{}},
+		},
+		"/buckets/b/a/b": {
+			{Name: "x", Attributes: &filer_pb.FuseAttributes{}},
+			{Name: "y", Attributes: &filer_pb.FuseAttributes{}},
+		},
+		"/buckets/b/b": {{Name: "trap", Attributes: &filer_pb.FuseAttributes{}}},
+	}}
+	for _, marker := range []string{"a/", "a/b", "a/b/", "a/b/x"} {
+		t.Run(marker, func(t *testing.T) {
+			cursor := &ListingCursor{maxKeys: 1000}
+			var seen []string
+			s3a := &S3ApiServer{}
+			_, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{
+				dir: "/buckets/b", marker: marker, bucket: "b",
+			}, cursor, func(dir string, entry *filer_pb.Entry) {
+				seen = append(seen, strings.TrimPrefix(dir, "/buckets/b")+"/"+entry.Name)
+				cursor.maxKeys--
+			})
+			want := []string{"/a/b/x", "/a/b/y", "/a/c", "/b/trap", "/z"}
+			if marker == "a/b/x" {
+				want = want[1:]
+			}
+			assert.NoError(t, err)
+			assert.Equal(t, want, seen)
+			assert.False(t, cursor.isTruncated)
+		})
+	}
+}
+
+type candidateLookupErrorFilerClient struct {
+	*testFilerClient
+	lookupErr error
+}
+
+func (c *candidateLookupErrorFilerClient) LookupDirectoryEntry(ctx context.Context, in *filer_pb.LookupDirectoryEntryRequest, opts ...grpc.CallOption) (*filer_pb.LookupDirectoryEntryResponse, error) {
+	if in.Directory == "/buckets/b" && in.Name == "a" {
+		return &filer_pb.LookupDirectoryEntryResponse{}, c.lookupErr
+	}
+	return c.testFilerClient.LookupDirectoryEntry(ctx, in, opts...)
+}
+
+func TestDoListFilerEntries_CandidateLookupErrors(t *testing.T) {
+	for _, tc := range []struct {
+		name      string
+		lookupErr error
+		wantError bool
+	}{
+		{name: "empty response"},
+		{name: "not found sentinel", lookupErr: filer_pb.ErrNotFound},
+		{name: "not found status", lookupErr: status.Error(codes.NotFound, "missing")},
+		{name: "legacy not found status", lookupErr: status.Error(codes.Unknown, filer_pb.ErrNotFound.Error())},
+		{name: "unavailable", lookupErr: status.Error(codes.Unavailable, "storage unavailable"), wantError: true},
+		{name: "unavailable with not found text", lookupErr: status.Error(codes.Unavailable, filer_pb.ErrNotFound.Error()), wantError: true},
+		{name: "deadline exceeded", lookupErr: context.DeadlineExceeded, wantError: true},
+		{name: "canceled", lookupErr: context.Canceled, wantError: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			client := &candidateLookupErrorFilerClient{
+				testFilerClient: &testFilerClient{entriesByDir: map[string][]*filer_pb.Entry{
+					"/buckets/b": {{Name: "a0", Attributes: &filer_pb.FuseAttributes{}}},
+				}},
+				lookupErr: tc.lookupErr,
+			}
+			cursor := &ListingCursor{maxKeys: 10}
+			var seen []string
+			s3a := &S3ApiServer{}
+			_, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{
+				dir: "/buckets/b", marker: "a-c", bucket: "b",
+			}, cursor, func(dir string, entry *filer_pb.Entry) {
+				seen = append(seen, entry.Name)
+				cursor.maxKeys--
+			})
+			if tc.wantError {
+				assert.ErrorIs(t, err, tc.lookupErr)
+				assert.ErrorContains(t, err, "/buckets/b/a")
+				assert.Empty(t, seen, "a failed candidate must not produce a partial successful page")
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, []string{"a0"}, seen)
+			}
+		})
+	}
+}
+
+func TestDoListFilerEntries_CandidateDirectoryMarkerEcho(t *testing.T) {
+	client := &markerEchoFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/b": {
+				{Name: "a", IsDirectory: true},
+				{Name: "b"},
+			},
+			"/buckets/b/a": {{Name: "x"}},
+		},
+		returnFollowing: true,
+	}
+	cursor := &ListingCursor{maxKeys: 1000}
+	var seen []string
+	s3a := &S3ApiServer{}
+	_, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{
+		dir: "/buckets/b", marker: "a", bucket: "b",
+	}, cursor, func(dir string, entry *filer_pb.Entry) {
+		seen = append(seen, strings.TrimPrefix(dir, "/buckets/b")+"/"+entry.Name)
+		cursor.maxKeys--
+	})
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"/a/x", "/b"}, seen)
+	assert.False(t, cursor.isTruncated)
+}

weed/s3api/s3api_object_handlers_list_token_test.go

@@ -347,11 +347,14 @@ func TestListObjectsV1MarkerEchoStaysXMLSafe(t *testing.T) {
 // the marker's first path segment and descends only into blocks that can still
 // hold keys past it.
 func TestListObjectsV2ResumeXMLUnsafePrefixWithSiblingDir(t *testing.T) {
+	// A key with children is stored as one promoted directory entry, so direct
+	// lookups and streamed listings must both see its object and subtree.
+	prefixObject := listTokenTestFile("a\x01b")
+	prefixObject.MarkPrefixObject()
 	filer := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{
 		"/buckets/test/x": {
 			{Name: "a\x01", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
-			listTokenTestFile("a\x01b"),
-			{Name: "a\x01b", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+			prefixObject,
 			listTokenTestFile("a\x01b0"),
 		},
 		"/buckets/test/x/a\x01": {