Fix: trim prefix slash in ListObjectVersionsHandler (#7919)
* Fix: trim prefix slash in ListObjectVersionsHandler * Add test for ListObjectVersions prefix handling Test validates that prefix normalization works correctly with and without leading slashes, ensuring the fix for /Veeam/Archive/ style prefixes. * Simplify prefix test to validate normalization logic The test now validates that the prefix normalization (TrimPrefix) works correctly and that normalized prefixes match paths as expected. This is a focused unit test that validates the core fix without requiring complex mocking of the filer client. * Enhance prefix test with full matchesPrefixFilter logic Added test cases for directory traversal including: - Directory matching with trailing slash - canDescend logic for recursive directory search - Full simulation of matchesPrefixFilter behavior This provides more comprehensive coverage of the prefix normalization fix and ensures it works correctly for both files and directories.
This commit is contained in:
@@ -431,3 +431,87 @@ func (c *customTestFilerClient) ListEntries(ctx context.Context, in *filer_pb.Li
|
|||||||
(*c.traversedDirs)[in.Directory] = true
|
(*c.traversedDirs)[in.Directory] = true
|
||||||
return c.testFilerClient.ListEntries(ctx, in, opts...)
|
return c.testFilerClient.ListEntries(ctx, in, opts...)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestListObjectVersions_PrefixWithLeadingSlash tests that prefixes with leading slashes work correctly
|
||||||
|
// This validates the fix for the bug where "/Veeam/Archive/" would fail to match relative paths
|
||||||
|
func TestListObjectVersions_PrefixWithLeadingSlash(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
inputPrefix string
|
||||||
|
expectedNormalized string
|
||||||
|
entryPath string
|
||||||
|
isDirectory bool
|
||||||
|
shouldMatch bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "Prefix without leading slash matches file",
|
||||||
|
inputPrefix: "Veeam/Archive/",
|
||||||
|
expectedNormalized: "Veeam/Archive/",
|
||||||
|
entryPath: "Veeam/Archive/file.txt",
|
||||||
|
isDirectory: false,
|
||||||
|
shouldMatch: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Prefix with leading slash (bug fix test) - normalized and matches file",
|
||||||
|
inputPrefix: "/Veeam/Archive/",
|
||||||
|
expectedNormalized: "Veeam/Archive/",
|
||||||
|
entryPath: "Veeam/Archive/file.txt",
|
||||||
|
isDirectory: false,
|
||||||
|
shouldMatch: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Normalized prefix matches subdirectory file",
|
||||||
|
inputPrefix: "/Veeam/",
|
||||||
|
expectedNormalized: "Veeam/",
|
||||||
|
entryPath: "Veeam/Backup/file.txt",
|
||||||
|
isDirectory: false,
|
||||||
|
shouldMatch: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Normalized prefix does not match different path",
|
||||||
|
inputPrefix: "/Veeam/Archive/",
|
||||||
|
expectedNormalized: "Veeam/Archive/",
|
||||||
|
entryPath: "Veeam/Backup/file.txt",
|
||||||
|
isDirectory: false,
|
||||||
|
shouldMatch: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Prefix with leading slash allows descending into directory",
|
||||||
|
inputPrefix: "/Veeam/Archive/",
|
||||||
|
expectedNormalized: "Veeam/Archive/",
|
||||||
|
entryPath: "Veeam",
|
||||||
|
isDirectory: true,
|
||||||
|
shouldMatch: true, // canDescend is true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Prefix with leading slash matches directory with trailing slash",
|
||||||
|
inputPrefix: "/Veeam/",
|
||||||
|
expectedNormalized: "Veeam/",
|
||||||
|
entryPath: "Veeam",
|
||||||
|
isDirectory: true,
|
||||||
|
shouldMatch: true, // isMatch becomes true
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
// This is the normalization logic from ListObjectVersionsHandler (the fix)
|
||||||
|
normalizedPrefix := strings.TrimPrefix(tt.inputPrefix, "/")
|
||||||
|
|
||||||
|
// Verify normalization worked correctly
|
||||||
|
assert.Equal(t, tt.expectedNormalized, normalizedPrefix,
|
||||||
|
"Prefix normalization should strip leading slash")
|
||||||
|
|
||||||
|
// This simulates the full matchesPrefixFilter logic used in findVersionsRecursively
|
||||||
|
isMatch := strings.HasPrefix(tt.entryPath, normalizedPrefix)
|
||||||
|
if !isMatch && tt.isDirectory {
|
||||||
|
isMatch = strings.HasPrefix(tt.entryPath+"/", normalizedPrefix)
|
||||||
|
}
|
||||||
|
canDescend := tt.isDirectory && strings.HasPrefix(normalizedPrefix, tt.entryPath)
|
||||||
|
matches := isMatch || canDescend
|
||||||
|
|
||||||
|
assert.Equal(t, tt.shouldMatch, matches,
|
||||||
|
"Normalized prefix should correctly match/not match the path based on full filter logic")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -985,7 +985,7 @@ func (s3a *S3ApiServer) ListObjectVersionsHandler(w http.ResponseWriter, r *http
|
|||||||
// Parse query parameters
|
// Parse query parameters
|
||||||
query := r.URL.Query()
|
query := r.URL.Query()
|
||||||
originalPrefix := query.Get("prefix") // Keep original prefix for response
|
originalPrefix := query.Get("prefix") // Keep original prefix for response
|
||||||
prefix := originalPrefix // Use for internal processing
|
prefix := strings.TrimPrefix(originalPrefix, "/")
|
||||||
// Note: prefix is used for filtering relative to bucket root, so no leading slash needed
|
// Note: prefix is used for filtering relative to bucket root, so no leading slash needed
|
||||||
|
|
||||||
keyMarker := query.Get("key-marker")
|
keyMarker := query.Get("key-marker")
|
||||||
|
|||||||
Reference in New Issue
Block a user