Fix S3 ListObjectsV2 recursion issue (#8347)
* Fix S3 ListObjectsV2 recursion issue (#8346) Removed aggressive Limit=1 optimization in doListFilerEntries that caused missed directory entries when prefix ended with a delimiter. Added regression tests to verify deep directory traversal. * Address PR comments: condense test comments
This commit is contained in:
@@ -520,9 +520,6 @@ func (s3a *S3ApiServer) doListFilerEntries(client filer_pb.SeaweedFilerClient, d
|
|||||||
StartFromFileName: marker,
|
StartFromFileName: marker,
|
||||||
InclusiveStartFrom: inclusiveStartFrom,
|
InclusiveStartFrom: inclusiveStartFrom,
|
||||||
}
|
}
|
||||||
if cursor.prefixEndsOnDelimiter {
|
|
||||||
request.Limit = uint32(1)
|
|
||||||
}
|
|
||||||
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
defer cancel()
|
defer cancel()
|
||||||
|
|||||||
@@ -55,6 +55,12 @@ func (c *testFilerClient) ListEntries(ctx context.Context, in *filer_pb.ListEntr
|
|||||||
}
|
}
|
||||||
entries = filtered
|
entries = filtered
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Respect Limit
|
||||||
|
if in.Limit > 0 && int(in.Limit) < len(entries) {
|
||||||
|
entries = entries[:in.Limit]
|
||||||
|
}
|
||||||
|
|
||||||
return &testListEntriesStream{entries: entries}, nil
|
return &testListEntriesStream{entries: entries}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -594,3 +600,83 @@ func TestObjectLevelListPermissions(t *testing.T) {
|
|||||||
t.Log("Object-level List permissions like 'List:bucket/prefix/*' now work correctly")
|
t.Log("Object-level List permissions like 'List:bucket/prefix/*' now work correctly")
|
||||||
t.Log("Middleware properly extracts prefix for permission validation")
|
t.Log("Middleware properly extracts prefix for permission validation")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestListObjectsV2_Regression(t *testing.T) {
|
||||||
|
// Reproduce issue: ListObjectsV2 without delimiter returns 0 objects even though files exist
|
||||||
|
// Structure: s3://reports/reports/[timestamp]/file
|
||||||
|
// Request: ListObjectsV2(Bucket='reports', Prefix='reports/')
|
||||||
|
|
||||||
|
s3a := &S3ApiServer{}
|
||||||
|
client := &testFilerClient{
|
||||||
|
entriesByDir: map[string][]*filer_pb.Entry{
|
||||||
|
"/buckets/reports": {
|
||||||
|
{Name: "reports", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
},
|
||||||
|
"/buckets/reports/reports": {
|
||||||
|
{Name: "01771152617961894200", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
},
|
||||||
|
"/buckets/reports/reports/01771152617961894200": {
|
||||||
|
{Name: "file1", IsDirectory: false, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// s3.list_objects_v2(Bucket='reports', Prefix='reports/')
|
||||||
|
// normalized: requestDir="", prefix="reports"
|
||||||
|
// doListFilerEntries called with dir="/buckets/reports", prefix="reports", delimiter=""
|
||||||
|
|
||||||
|
cursor := &ListingCursor{maxKeys: 1000, prefixEndsOnDelimiter: true} // set based on "reports/" original prefix
|
||||||
|
var results []string
|
||||||
|
|
||||||
|
// Call doListFilerEntries directly to unit test listing logic in isolation,
|
||||||
|
// simulating parameters passed from listFilerEntries for prefix "reports/".
|
||||||
|
|
||||||
|
_, err := s3a.doListFilerEntries(client, "/buckets/reports", "reports", cursor, "", "", false, "reports", func(dir string, entry *filer_pb.Entry) {
|
||||||
|
if !entry.IsDirectory {
|
||||||
|
results = append(results, entry.Name)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assert.Contains(t, results, "file1", "Should return the nested file")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestListObjectsV2_Regression_Sorting(t *testing.T) {
|
||||||
|
// Verify that listing logic correctly finds the target directory even when
|
||||||
|
// other entries with a similar prefix are returned first by the filer,
|
||||||
|
// a scenario where the removed Limit=1 optimization would fail.
|
||||||
|
|
||||||
|
s3a := &S3ApiServer{}
|
||||||
|
client := &testFilerClient{
|
||||||
|
entriesByDir: map[string][]*filer_pb.Entry{
|
||||||
|
"/buckets/reports": {
|
||||||
|
{Name: "reports-archive", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
{Name: "reports", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
},
|
||||||
|
"/buckets/reports/reports": {
|
||||||
|
{Name: "01771152617961894200", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
},
|
||||||
|
"/buckets/reports/reports/01771152617961894200": {
|
||||||
|
{Name: "file1", IsDirectory: false, Attributes: &filer_pb.FuseAttributes{}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// This cursor setup mimics what happens in listFilerEntries
|
||||||
|
cursor := &ListingCursor{maxKeys: 1000, prefixEndsOnDelimiter: true}
|
||||||
|
var results []string
|
||||||
|
|
||||||
|
// Without the fix, Limit=1 would cause the lister to stop after "reports-archive",
|
||||||
|
// missing the intended "reports" directory.
|
||||||
|
|
||||||
|
_, err := s3a.doListFilerEntries(client, "/buckets/reports", "reports", cursor, "", "", false, "reports", func(dir string, entry *filer_pb.Entry) {
|
||||||
|
if !entry.IsDirectory {
|
||||||
|
results = append(results, entry.Name)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
assert.NoError(t, err)
|
||||||
|
// With Limit=1, this fails because it only sees "reports-archive"
|
||||||
|
// With fix, it sees both and processes "reports"
|
||||||
|
assert.Contains(t, results, "file1", "Should return the nested file even if 'reports' directory is not the first match")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user