From b034cf188e5cef98789eb2935d382cda58f80422 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 30 Dec 2025 14:54:37 -0800 Subject: [PATCH] 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. --- ...api_object_handlers_list_versioned_test.go | 84 +++++++++++++++++++ weed/s3api/s3api_object_versioning.go | 2 +- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/weed/s3api/s3api_object_handlers_list_versioned_test.go b/weed/s3api/s3api_object_handlers_list_versioned_test.go index 8252dc4a9..fdadcfe57 100644 --- a/weed/s3api/s3api_object_handlers_list_versioned_test.go +++ b/weed/s3api/s3api_object_handlers_list_versioned_test.go @@ -431,3 +431,87 @@ func (c *customTestFilerClient) ListEntries(ctx context.Context, in *filer_pb.Li (*c.traversedDirs)[in.Directory] = true 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") + }) + } +} diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index 3fadc46cb..6221eac1b 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -985,7 +985,7 @@ func (s3a *S3ApiServer) ListObjectVersionsHandler(w http.ResponseWriter, r *http // Parse query parameters query := r.URL.Query() 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 keyMarker := query.Get("key-marker")