Fix versioning list only (#7015)

* fix listing objects

* address comments

* Update weed/s3api/s3api_object_versioning.go

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update test/s3/versioning/s3_directory_versioning_test.go

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
Chris Lu 2025-07-21 10:35:21 -07:00 committed by GitHub
parent c196d03951
commit 3a5ee18265
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 108 additions and 3 deletions

View File

@ -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

View File

@ -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)
}