mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-19 00:01:31 +00:00
address comments
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user