mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 04:06:44 +00:00
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 <chris.lu@gmail.com>
This commit is contained in:
@@ -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 != "" {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user