From 0dd33bd7ec012096dffcdfe66b2c5eb2cb069edb Mon Sep 17 00:00:00 2001 From: hsdfat <118717478+hsdfat@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:24:44 +0700 Subject: [PATCH] s3api: fix ListObjectsV2 dropping objects under a partial prefix (#10656) * s3api: fix ListObjectsV2 dropping objects under a partial prefix ListObjectsV2 with a prefix that names only part of a directory name - "data/a" matching both "data/a/..." and "data/ab/..." - dropped objects when paginating with a small max-keys, reporting IsTruncated=false before all matching keys had been returned. On resume, normalizePrefixMarker took the listing directory from the continuation marker ("data/a/") and discarded the "a" name prefix, so once the marker's subtree was exhausted the sibling directories that also matched the prefix were never listed. Resolve the listing directory and name prefix from the prefix instead of the marker, so siblings sharing the name prefix are still listed after the marker's subtree. Fixes #10652 * s3api: cover a sibling-directory marker in the partial prefix tests * s3api: resume a truncated delimited listing from the emitted CommonPrefix Rebuilding the continuation token from the request dir plus the listing prefix assumes that prefix names a directory. With a partial name prefix like "data/a" it produced "data/a/ab/" for the sibling prefix "data/ab/", so the next page resumed in the wrong subtree and repeated it forever. --------- Co-authored-by: Chris Lu --- weed/s3api/s3api_object_handlers_list.go | 45 +++++++------------ weed/s3api/s3api_object_handlers_list_test.go | 35 ++++++++++++--- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 11d8c9fa1..23785cb31 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -301,7 +301,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq // check filer err = s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { var lastEntryWasCommonPrefix bool - var lastCommonPrefixName string + var lastCommonPrefix string // Hoist versioning check out of per-entry callback versioningState, _ := s3a.getVersioningState(bucket) @@ -361,7 +361,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq delimitedPrefix := originalPrefix + delimitedPath[0] + delimiter // Check if this CommonPrefix already exists - if !lastEntryWasCommonPrefix || lastCommonPrefixName != delimitedPath[0] { + if !lastEntryWasCommonPrefix || lastCommonPrefix != delimitedPrefix { // New CommonPrefix found commonPrefixes = append(commonPrefixes, PrefixEntry{ Prefix: delimitedPrefix, @@ -369,7 +369,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq cursor.maxKeys-- delimiterFound = true lastEntryWasCommonPrefix = true - lastCommonPrefixName = delimitedPath[0] + lastCommonPrefix = delimitedPrefix } else { // This directory object belongs to an existing CommonPrefix, skip it delimiterFound = true @@ -391,13 +391,14 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq } else if delimiter != "" { // A response can contain CommonPrefixes only if you specify a delimiter. // Use raw dir and entry.Name (not encoded) to ensure consistent handling // Encoding will be applied after sorting if encodingTypeUrl is set + dirPrefix := fmt.Sprintf("%s/%s/", dir, entry.Name)[len(bucketPrefix):] commonPrefixes = append(commonPrefixes, PrefixEntry{ - Prefix: fmt.Sprintf("%s/%s/", dir, entry.Name)[len(bucketPrefix):], + Prefix: dirPrefix, }) //All of the keys (up to 1,000) rolled up into a common prefix count as a single return when calculating the number of returns. cursor.maxKeys-- lastEntryWasCommonPrefix = true - lastCommonPrefixName = entry.Name + lastCommonPrefix = dirPrefix } } else { var delimiterFound bool @@ -429,7 +430,7 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq cursor.maxKeys-- delimiterFound = true lastEntryWasCommonPrefix = true - lastCommonPrefixName = delimitedPath[0] + lastCommonPrefix = delimitedPrefix } else { // This object belongs to an existing CommonPrefix, skip it // but continue processing to maintain correct flow @@ -454,9 +455,8 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq return doErr } - // Adjust nextMarker for CommonPrefixes to include trailing slash (AWS S3 compliance) if cursor.isTruncated { - nextMarker = buildTruncatedNextMarker(requestDir, prefix, nextMarker, lastEntryWasCommonPrefix, lastCommonPrefixName) + nextMarker = buildTruncatedNextMarker(requestDir, nextMarker, lastEntryWasCommonPrefix, lastCommonPrefix) } if cursor.isTruncated { @@ -535,13 +535,9 @@ func normalizePrefixMarker(prefix, marker string) (alignedDir, alignedPrefix, al // something wrong return "", prefix, marker } - if strings.HasPrefix(marker, prefix+"/") { - alignedDir = prefix - alignedPrefix = "" - alignedMarker = marker[len(alignedDir)+1:] - return - } - + // 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 + // subtree would drop. alignedDir, alignedPrefix = toDirAndName(prefix) if alignedDir != "" { alignedMarker = marker[len(alignedDir)+1:] @@ -571,19 +567,12 @@ func toParentAndDescendants(dirAndName string) (dir, name string) { return } -func buildTruncatedNextMarker(requestDir, prefix, nextMarker string, lastEntryWasCommonPrefix bool, lastCommonPrefixName string) string { - if lastEntryWasCommonPrefix && lastCommonPrefixName != "" { - // For CommonPrefixes, NextMarker should include the trailing slash - if requestDir != "" { - if prefix != "" { - return requestDir + "/" + prefix + "/" + lastCommonPrefixName + "/" - } - return requestDir + "/" + lastCommonPrefixName + "/" - } - if prefix != "" { - return prefix + "/" + lastCommonPrefixName + "/" - } - return lastCommonPrefixName + "/" +func buildTruncatedNextMarker(requestDir, nextMarker string, lastEntryWasCommonPrefix bool, lastCommonPrefix string) string { + // The emitted CommonPrefix is already the full key path with its trailing delimiter. + // Rebuilding it from requestDir plus the listing prefix breaks when that prefix is a + // partial name, not a directory. + if lastEntryWasCommonPrefix && lastCommonPrefix != "" { + return lastCommonPrefix } if requestDir != "" { diff --git a/weed/s3api/s3api_object_handlers_list_test.go b/weed/s3api/s3api_object_handlers_list_test.go index 8d5d990cd..9cffaee85 100644 --- a/weed/s3api/s3api_object_handlers_list_test.go +++ b/weed/s3api/s3api_object_handlers_list_test.go @@ -207,6 +207,27 @@ func Test_normalizePrefixMarker(t *testing.T) { "parent", "parentDir/data/0e/0e149049a2137b0cc12e", }, + {"partial name prefix, marker resumes inside a matching subdirectory", + args{"data/a", + "data/a/1"}, + "data", + "a", + "a/1", + }, + {"partial name prefix, marker resumes inside a matching sibling directory", + args{"data/a", + "data/ab/1"}, + "data", + "a", + "ab/1", + }, + {"top-level partial name prefix, marker resumes inside a matching subdirectory", + args{"a", + "a/1"}, + "", + "a", + "a/1", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -220,22 +241,26 @@ func Test_normalizePrefixMarker(t *testing.T) { func TestBuildTruncatedNextMarker(t *testing.T) { t.Run("does not duplicate prefix segment in next continuation token", func(t *testing.T) { - prefix := "export_2026-02-10_17-00-23" nextMarker := "export_2026-02-10_17-00-23/4156000e.jpg" - actual := buildTruncatedNextMarker("xemu", prefix, nextMarker, false, "") + actual := buildTruncatedNextMarker("xemu", nextMarker, false, "") assert.Equal(t, "xemu/export_2026-02-10_17-00-23/4156000e.jpg", actual) }) t.Run("keeps common prefix marker trailing slash", func(t *testing.T) { - actual := buildTruncatedNextMarker("xemu", "export_2026-02-10_17-00-23", "", true, "nested") + actual := buildTruncatedNextMarker("xemu", "", true, "xemu/export_2026-02-10_17-00-23/nested/") assert.Equal(t, "xemu/export_2026-02-10_17-00-23/nested/", actual) }) - t.Run("includes prefix for common prefix marker when request dir is empty", func(t *testing.T) { - actual := buildTruncatedNextMarker("", "foo", "", true, "bar") + t.Run("keeps common prefix marker when request dir is empty", func(t *testing.T) { + actual := buildTruncatedNextMarker("", "", true, "foo/bar/") assert.Equal(t, "foo/bar/", actual) }) + + t.Run("does not fold a partial name prefix into the common prefix marker", func(t *testing.T) { + actual := buildTruncatedNextMarker("data", "", true, "data/ab/") + assert.Equal(t, "data/ab/", actual) + }) } func TestAllowUnorderedParameterValidation(t *testing.T) {