aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorchrislu <chris.lu@gmail.com>2025-07-21 08:51:22 -0700
committerchrislu <chris.lu@gmail.com>2025-07-21 08:51:22 -0700
commit92cedd4637698cfdc247bb3f01e8bb5061961566 (patch)
tree9c3c270ad35f849c7d589f20e62401173ea0fe24
parent8ee24426b33130bd917c82b51ab73ce4199f06be (diff)
downloadseaweedfs-92cedd4637698cfdc247bb3f01e8bb5061961566.tar.xz
seaweedfs-92cedd4637698cfdc247bb3f01e8bb5061961566.zip
address comments
-rw-r--r--test/s3/versioning/s3_directory_versioning_test.go85
-rw-r--r--weed/s3api/s3api_object_versioning.go19
2 files changed, 95 insertions, 9 deletions
diff --git a/test/s3/versioning/s3_directory_versioning_test.go b/test/s3/versioning/s3_directory_versioning_test.go
index 7874bc055..bc1928423 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.Background(), &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 97102b9dc..7d9aa43a7 100644
--- a/weed/s3api/s3api_object_versioning.go
+++ b/weed/s3api/s3api_object_versioning.go
@@ -263,20 +263,21 @@ func (s3a *S3ApiServer) findVersionsRecursively(currentPath, relativePath string
entryPath := path.Join(relativePath, entry.Name)
// Skip if this doesn't match the prefix filter
- // Normalize both entryPath and prefix to handle leading slash differences
if prefix != "" {
normalizedPrefix := strings.TrimPrefix(prefix, "/")
if normalizedPrefix != "" {
- // For directories, also check if the directory name with trailing slash matches the prefix
- entryPathWithSlash := entryPath
- if entry.IsDirectory && !strings.HasSuffix(entryPathWithSlash, "/") {
- entryPathWithSlash += "/"
+ // An entry is a candidate if:
+ // 1. Its path is prefixed by normalizedPrefix.
+ // 2. It is a directory that is a prefix of normalizedPrefix (so we can descend into it).
+ isPrefixed := strings.HasPrefix(entryPath, normalizedPrefix)
+ if !isPrefixed && entry.IsDirectory {
+ // For directories, also check with a trailing slash.
+ isPrefixed = strings.HasPrefix(entryPath+"/", normalizedPrefix)
}
- // Check if either the entry path or the path with slash matches the prefix
- if !strings.HasPrefix(entryPath, normalizedPrefix) &&
- !strings.HasPrefix(entryPathWithSlash, normalizedPrefix) &&
- !strings.HasPrefix(normalizedPrefix, entryPath) {
+ canDescend := entry.IsDirectory && strings.HasPrefix(normalizedPrefix, entryPath)
+
+ if !isPrefixed && !canDescend {
continue
}
}