listing

This reverts commit 8ee24426b3.

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,251 @@ 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")
+}
+
+// TestNextMarkerDelimiterFix tests the NextMarker behavior with delimiters for directory prefixes
+// This addresses the test failures where NextMarker should include trailing slash for CommonPrefixes
+func TestNextMarkerDelimiterFix(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)
+
+	// Create test objects that match the failing test pattern
+	testObjects := []string{
+		"asdf",
+		"boo/bar",
+		"boo/baz/xyzzy",
+		"cquux/thud",
+		"cquux/bla",
+	}
+
+	for _, objectKey := range testObjects {
+		_, err = s3Client.PutObject(context.Background(), &s3.PutObjectInput{
+			Bucket: aws.String(bucketName),
+			Key:    aws.String(objectKey),
+			Body:   strings.NewReader("test content for " + objectKey),
+		})
+		require.NoError(t, err)
+	}
+
+	// Test the specific failing case: list with delimiter "/" and maxKeys=1
+	// This should return objects/prefixes one at a time with correct NextMarker
+
+	// First call: should return "asdf" with NextMarker="asdf"
+	listResponse, err := s3Client.ListObjects(context.Background(), &s3.ListObjectsInput{
+		Bucket:    aws.String(bucketName),
+		Delimiter: aws.String("/"),
+		MaxKeys:   aws.Int32(1),
+	})
+	require.NoError(t, err)
+
+	assert.True(t, *listResponse.IsTruncated, "First response should be truncated")
+	assert.Len(t, listResponse.Contents, 1, "Should return exactly 1 object")
+	assert.Equal(t, "asdf", aws.ToString(listResponse.Contents[0].Key), "First object should be 'asdf'")
+	assert.Equal(t, "asdf", aws.ToString(listResponse.NextMarker), "NextMarker should be 'asdf'")
+
+	// Second call: continue from "asdf", should return CommonPrefix "boo/" with NextMarker="boo/"
+	listResponse, err = s3Client.ListObjects(context.Background(), &s3.ListObjectsInput{
+		Bucket:    aws.String(bucketName),
+		Delimiter: aws.String("/"),
+		Marker:    listResponse.NextMarker,
+		MaxKeys:   aws.Int32(1),
+	})
+	require.NoError(t, err)
+
+	assert.True(t, *listResponse.IsTruncated, "Second response should be truncated")
+	assert.Len(t, listResponse.Contents, 0, "Should return no direct objects")
+	assert.Len(t, listResponse.CommonPrefixes, 1, "Should return exactly 1 CommonPrefix")
+	assert.Equal(t, "boo/", aws.ToString(listResponse.CommonPrefixes[0].Prefix), "CommonPrefix should be 'boo/'")
+
+	// This is the critical assertion that was failing:
+	assert.Equal(t, "boo/", aws.ToString(listResponse.NextMarker), "NextMarker should be 'boo/' (with trailing slash)")
+
+	// Third call: continue from "boo/", should return CommonPrefix "cquux/"
+	listResponse, err = s3Client.ListObjects(context.Background(), &s3.ListObjectsInput{
+		Bucket:    aws.String(bucketName),
+		Delimiter: aws.String("/"),
+		Marker:    listResponse.NextMarker,
+		MaxKeys:   aws.Int32(1),
+	})
+	require.NoError(t, err)
+
+	assert.False(t, *listResponse.IsTruncated, "Third response should not be truncated")
+	assert.Len(t, listResponse.Contents, 0, "Should return no direct objects")
+	assert.Len(t, listResponse.CommonPrefixes, 1, "Should return exactly 1 CommonPrefix")
+	assert.Equal(t, "cquux/", aws.ToString(listResponse.CommonPrefixes[0].Prefix), "CommonPrefix should be 'cquux/'")
+
+	t.Logf("✅ NextMarker correctly includes trailing slash for CommonPrefixes")
+}
+
+// TestIsTruncatedLogicFix tests the IsTruncated flag behavior for list operations
+// This addresses the test failures where IsTruncated was incorrectly set to true
+func TestIsTruncatedLogicFix(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)
+
+	// Create test objects that match the failing test pattern
+	testObjects := []string{
+		"asdf",
+		"boo/bar",
+		"boo/baz/xyzzy",
+		"cquux/thud",
+		"cquux/bla",
+	}
+
+	for _, objectKey := range testObjects {
+		_, err = s3Client.PutObject(context.Background(), &s3.PutObjectInput{
+			Bucket: aws.String(bucketName),
+			Key:    aws.String(objectKey),
+			Body:   strings.NewReader("test content for " + objectKey),
+		})
+		require.NoError(t, err)
+	}
+
+	// Test ListObjectsV2 with the exact failing pattern from the s3tests
+	// With delimiter="/", this creates: 'asdf' (object), 'boo/' (CommonPrefix), 'cquux/' (CommonPrefix)
+	// So total of 3 "items" to return
+
+	// First call: MaxKeys=1, should return 'asdf', IsTruncated=true (2 more items)
+	listResponse, err := s3Client.ListObjectsV2(context.Background(), &s3.ListObjectsV2Input{
+		Bucket:    aws.String(bucketName),
+		Delimiter: aws.String("/"),
+		MaxKeys:   aws.Int32(1),
+	})
+	require.NoError(t, err)
+
+	assert.True(t, *listResponse.IsTruncated, "First response should be truncated")
+	assert.Equal(t, 1, listResponse.KeyCount, "Should return 1 item")
+	assert.Len(t, listResponse.Contents, 1, "Should return 1 object")
+	assert.Equal(t, "asdf", aws.ToString(listResponse.Contents[0].Key), "First object should be 'asdf'")
+
+	// Second call: continue with NextContinuationToken, MaxKeys=1, should return 'boo/', IsTruncated=true (1 more item)
+	listResponse, err = s3Client.ListObjectsV2(context.Background(), &s3.ListObjectsV2Input{
+		Bucket:            aws.String(bucketName),
+		Delimiter:         aws.String("/"),
+		MaxKeys:           aws.Int32(1),
+		ContinuationToken: listResponse.NextContinuationToken,
+	})
+	require.NoError(t, err)
+
+	assert.True(t, *listResponse.IsTruncated, "Second response should be truncated")
+	assert.Equal(t, 1, listResponse.KeyCount, "Should return 1 item")
+	assert.Len(t, listResponse.CommonPrefixes, 1, "Should return 1 CommonPrefix")
+	assert.Equal(t, "boo/", aws.ToString(listResponse.CommonPrefixes[0].Prefix), "CommonPrefix should be 'boo/'")
+
+	// Third call: continue with NextContinuationToken, MaxKeys=1, should return 'cquux/', IsTruncated=FALSE (no more items)
+	listResponse, err = s3Client.ListObjectsV2(context.Background(), &s3.ListObjectsV2Input{
+		Bucket:            aws.String(bucketName),
+		Delimiter:         aws.String("/"),
+		MaxKeys:           aws.Int32(1),
+		ContinuationToken: listResponse.NextContinuationToken,
+	})
+	require.NoError(t, err)
+
+	// This is the critical assertion that was failing before the fix
+	assert.False(t, *listResponse.IsTruncated, "Third response should NOT be truncated (this was the bug)")
+	assert.Equal(t, 1, listResponse.KeyCount, "Should return 1 item")
+	assert.Len(t, listResponse.CommonPrefixes, 1, "Should return 1 CommonPrefix")
+	assert.Equal(t, "cquux/", aws.ToString(listResponse.CommonPrefixes[0].Prefix), "CommonPrefix should be 'cquux/'")
+	assert.Nil(t, listResponse.NextContinuationToken, "NextContinuationToken should be nil when not truncated")
+
+	t.Logf("✅ IsTruncated logic correctly identifies when there are no more results")
+}
+
 // Helper function to setup S3 client
 func setupS3Client(t *testing.T) *s3.Client {
 	// S3TestConfig holds configuration for S3 tests

weed/s3api/s3api_object_handlers.go

@@ -211,15 +211,13 @@ func newListEntry(entry *filer_pb.Entry, key string, dir string, name string, bu
 			ownerID = s3_constants.AccountAnonymousId
 			displayName = "anonymous"
 		} else {
-			// Try to resolve display name from IAM system
-			displayName = "unknown"
-			// Note: IAM resolution would require access to the S3ApiServer instance
-			// For now, use a simple fallback or could be enhanced later
-		}
+			// Use the ownerID as displayName if no better option is available
+			displayName = ownerID
 
-		// Additional fallback to file system username if available and no display name resolved
-		if displayName == "unknown" && entry.Attributes.UserName != "" {
-			displayName = entry.Attributes.UserName
+			// Additional fallback to file system username if available and different from ownerID
+			if entry.Attributes.UserName != "" && entry.Attributes.UserName != ownerID {
+				displayName = entry.Attributes.UserName
+			}
 		}
 
 		listEntry.Owner = CanonicalUser{

weed/s3api/s3api_object_handlers_list.go

@@ -148,10 +148,32 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m
 		prefixEndsOnDelimiter: strings.HasSuffix(originalPrefix, "/") && len(originalMarker) == 0,
 	}
 
+	// Special case: when maxKeys = 0, return empty results immediately
+	if maxKeys == 0 {
+		response = ListBucketResult{
+			Name:           bucket,
+			Prefix:         originalPrefix,
+			Marker:         originalMarker,
+			NextMarker:     "",
+			MaxKeys:        int(maxKeys),
+			Delimiter:      delimiter,
+			IsTruncated:    false,
+			Contents:       contents,
+			CommonPrefixes: commonPrefixes,
+		}
+		if encodingTypeUrl {
+			response.EncodingType = s3.EncodingTypeUrl
+		}
+		return
+	}
+
 	// check filer
 	err = s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
 		for {
 			empty := true
+			var lastEntryWasCommonPrefix bool
+			var lastCommonPrefixName string
+
 			nextMarker, doErr = s3a.doListFilerEntries(client, reqDir, prefix, cursor, marker, delimiter, false, func(dir string, entry *filer_pb.Entry) {
 				empty = false
 				dirName, entryName, prefixName := entryUrlEncode(dir, entry.Name, encodingTypeUrl)
@@ -159,6 +181,7 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m
 					if entry.IsDirectoryKeyObject() {
 						contents = append(contents, newListEntry(entry, "", dirName, entryName, bucketPrefix, fetchOwner, true, false))
 						cursor.maxKeys--
+						lastEntryWasCommonPrefix = false
 						// https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html
 					} else if delimiter == "/" { // A response can contain CommonPrefixes only if you specify a delimiter.
 						commonPrefixes = append(commonPrefixes, PrefixEntry{
@@ -166,6 +189,8 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m
 						})
 						//All of the keys (up to 1,000) rolled up into a common prefix count as a single return when calculating the number of returns.
 						cursor.maxKeys--
+						lastEntryWasCommonPrefix = true
+						lastCommonPrefixName = entry.Name
 					}
 				} else {
 					var delimiterFound bool
@@ -196,12 +221,15 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m
 								})
 								cursor.maxKeys--
 								delimiterFound = true
+								lastEntryWasCommonPrefix = true
+								lastCommonPrefixName = delimitedPath[0]
 							}
 						}
 					}
 					if !delimiterFound {
 						contents = append(contents, newListEntry(entry, "", dirName, entryName, bucketPrefix, fetchOwner, false, false))
 						cursor.maxKeys--
+						lastEntryWasCommonPrefix = false
 					}
 				}
 			})
@@ -209,10 +237,21 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m
 				return doErr
 			}
 
-			if cursor.isTruncated {
+			// Adjust nextMarker for CommonPrefixes to include trailing slash (AWS S3 compliance)
+			if cursor.isTruncated && lastEntryWasCommonPrefix && lastCommonPrefixName != "" {
+				// For CommonPrefixes, NextMarker should include the trailing slash
+				if requestDir != "" {
+					nextMarker = requestDir + "/" + lastCommonPrefixName + "/"
+				} else {
+					nextMarker = lastCommonPrefixName + "/"
+				}
+			} else if cursor.isTruncated {
 				if requestDir != "" {
 					nextMarker = requestDir + "/" + nextMarker
 				}
+			}
+
+			if cursor.isTruncated {
 				break
 			} else if empty || strings.HasSuffix(originalPrefix, "/") {
 				nextMarker = ""
@@ -371,8 +410,18 @@ func (s3a *S3ApiServer) doListFilerEntries(client filer_pb.SeaweedFilerClient, d
 			}
 		}
 		if cursor.maxKeys <= 0 {
-			cursor.isTruncated = true
-			continue
+			// Check if there are more entries available by trying to peek at the next one
+			_, nextRecvErr := stream.Recv()
+			if nextRecvErr == nil {
+				// There is another entry available, so we're truncated
+				cursor.isTruncated = true
+			} else if nextRecvErr != io.EOF {
+				// Some other error occurred
+				err = fmt.Errorf("peeking next entry: %v", nextRecvErr)
+				return
+			}
+			// If nextRecvErr == io.EOF, there are no more entries, so isTruncated remains false
+			break
 		}
 		entry := resp.Entry
 		nextMarker = entry.Name
@@ -442,6 +491,7 @@ func (s3a *S3ApiServer) doListFilerEntries(client filer_pb.SeaweedFilerClient, d
 	// Create logical entries for objects that have .versions directories
 	for _, versionsDir := range versionsDirs {
 		if cursor.maxKeys <= 0 {
+			// We have versioned objects remaining but can't process them due to limit
 			cursor.isTruncated = true
 			break
 		}

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 prefix != "" {
+			normalizedPrefix := strings.TrimPrefix(prefix, "/")
+			if normalizedPrefix != "" {
+				// 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)
+				}
+
+				canDescend := entry.IsDirectory && strings.HasPrefix(normalizedPrefix, entryPath)
+
+				if !isPrefixed && !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)
 }