diff options
| -rw-r--r-- | test/s3/versioning/s3_directory_versioning_test.go | 85 | ||||
| -rw-r--r-- | weed/s3api/s3api_object_versioning.go | 26 |
2 files changed, 108 insertions, 3 deletions
diff --git a/test/s3/versioning/s3_directory_versioning_test.go b/test/s3/versioning/s3_directory_versioning_test.go index 7874bc055..096065506 100644 --- a/test/s3/versioning/s3_directory_versioning_test.go +++ b/test/s3/versioning/s3_directory_versioning_test.go @@ -3,9 +3,11 @@ package s3api import ( "context" "fmt" + "sort" "strings" "sync" "testing" + "time" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" @@ -711,6 +713,89 @@ func TestVersionedObjectListBehavior(t *testing.T) { t.Logf("Successfully verified versioned object list behavior") } +// TestPrefixFilteringLogic tests the prefix filtering logic fix for list object versions +// This addresses the issue raised by gemini-code-assist bot where files could be incorrectly included +func TestPrefixFilteringLogic(t *testing.T) { + s3Client := setupS3Client(t) + bucketName := "test-bucket-" + fmt.Sprintf("%d", time.Now().UnixNano()) + + // Create bucket + _, err := s3Client.CreateBucket(context.TODO(), &s3.CreateBucketInput{ + Bucket: aws.String(bucketName), + }) + require.NoError(t, err) + defer cleanupBucket(t, s3Client, bucketName) + + // Enable versioning + _, err = s3Client.PutBucketVersioning(context.Background(), &s3.PutBucketVersioningInput{ + Bucket: aws.String(bucketName), + VersioningConfiguration: &types.VersioningConfiguration{ + Status: types.BucketVersioningStatusEnabled, + }, + }) + require.NoError(t, err) + + // Create test files that could trigger the edge case: + // - File "a" (which should NOT be included when searching for prefix "a/b") + // - File "a/b" (which SHOULD be included when searching for prefix "a/b") + _, err = s3Client.PutObject(context.Background(), &s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String("a"), + Body: strings.NewReader("content of file a"), + }) + require.NoError(t, err) + + _, err = s3Client.PutObject(context.Background(), &s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String("a/b"), + Body: strings.NewReader("content of file a/b"), + }) + require.NoError(t, err) + + // Test list-object-versions with prefix "a/b" - should NOT include file "a" + versionsResponse, err := s3Client.ListObjectVersions(context.Background(), &s3.ListObjectVersionsInput{ + Bucket: aws.String(bucketName), + Prefix: aws.String("a/b"), + }) + require.NoError(t, err) + + // Verify that only "a/b" is returned, not "a" + require.Len(t, versionsResponse.Versions, 1, "Should only find one version matching prefix 'a/b'") + assert.Equal(t, "a/b", aws.ToString(versionsResponse.Versions[0].Key), "Should only return 'a/b', not 'a'") + + // Test list-object-versions with prefix "a/" - should include "a/b" but not "a" + versionsResponse, err = s3Client.ListObjectVersions(context.Background(), &s3.ListObjectVersionsInput{ + Bucket: aws.String(bucketName), + Prefix: aws.String("a/"), + }) + require.NoError(t, err) + + // Verify that only "a/b" is returned, not "a" + require.Len(t, versionsResponse.Versions, 1, "Should only find one version matching prefix 'a/'") + assert.Equal(t, "a/b", aws.ToString(versionsResponse.Versions[0].Key), "Should only return 'a/b', not 'a'") + + // Test list-object-versions with prefix "a" - should include both "a" and "a/b" + versionsResponse, err = s3Client.ListObjectVersions(context.Background(), &s3.ListObjectVersionsInput{ + Bucket: aws.String(bucketName), + Prefix: aws.String("a"), + }) + require.NoError(t, err) + + // Should find both files + require.Len(t, versionsResponse.Versions, 2, "Should find both versions matching prefix 'a'") + + // Extract keys and sort them for predictable comparison + var keys []string + for _, version := range versionsResponse.Versions { + keys = append(keys, aws.ToString(version.Key)) + } + sort.Strings(keys) + + assert.Equal(t, []string{"a", "a/b"}, keys, "Should return both 'a' and 'a/b'") + + t.Logf("✅ Prefix filtering logic correctly handles edge cases") +} + // Helper function to setup S3 client func setupS3Client(t *testing.T) *s3.Client { // S3TestConfig holds configuration for S3 tests diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index e6b6f975b..e9802d71c 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -263,8 +263,24 @@ func (s3a *S3ApiServer) findVersionsRecursively(currentPath, relativePath string entryPath := path.Join(relativePath, entry.Name) // Skip if this doesn't match the prefix filter - if prefix != "" && !strings.HasPrefix(entryPath, strings.TrimPrefix(prefix, "/")) { - continue + if normalizedPrefix := strings.TrimPrefix(prefix, "/"); normalizedPrefix != "" { + // An entry is a candidate if: + // 1. Its path is a match for the prefix. + // 2. It is a directory that is an ancestor of the prefix path, so we must descend into it. + + // Condition 1: The entry's path starts with the prefix. + isMatch := strings.HasPrefix(entryPath, normalizedPrefix) + if !isMatch && entry.IsDirectory { + // Also check if a directory entry matches a directory-style prefix (e.g., prefix "a/", entry "a"). + isMatch = strings.HasPrefix(entryPath+"/", normalizedPrefix) + } + + // Condition 2: The prefix path starts with the entry's path (and it's a directory). + canDescend := entry.IsDirectory && strings.HasPrefix(normalizedPrefix, entryPath) + + if !isMatch && !canDescend { + continue + } } if entry.IsDirectory { @@ -715,7 +731,8 @@ func (s3a *S3ApiServer) ListObjectVersionsHandler(w http.ResponseWriter, r *http // Parse query parameters query := r.URL.Query() - prefix := query.Get("prefix") + originalPrefix := query.Get("prefix") // Keep original prefix for response + prefix := originalPrefix // Use for internal processing if prefix != "" && !strings.HasPrefix(prefix, "/") { prefix = "/" + prefix } @@ -740,6 +757,9 @@ func (s3a *S3ApiServer) ListObjectVersionsHandler(w http.ResponseWriter, r *http return } + // Set the original prefix in the response (not the normalized internal prefix) + result.Prefix = originalPrefix + writeSuccessResponseXML(w, r, result) } |