Browse Source

fix: directory incorrectly listed as object in S3 ListObjects (#7939)

* fix: directory incorrectly listed as object in S3 ListObjects

Regular directories (without MIME type) were only added to CommonPrefixes
when delimiter was exactly '/'. This caused directories to be silently
skipped for other delimiter values.

Changed the condition from 'delimiter == "/"' to 'delimiter != ""' to
ensure directories are correctly added to CommonPrefixes for any delimiter.

Fixes issue where directories like 'data/file.vhd' were being returned as
objects instead of prefixes in ListObjects responses.

* fix: complete the directory listing fix for all delimiters

Address reviewer feedback:
- Changed doListFilerEntries line 549 from 'delimiter != "/"' to 'delimiter == ""'
  This ensures directories are yielded to the callback for ANY delimiter, not just "/"
- Parameterized test to verify fix works with multiple delimiters (/, _, :)

The previous fix only addressed line 260 but line 549 was still causing
recursion for non-"/" delimiters, preventing directories from being
added to CommonPrefixes.

* docs: update test comment to reflect multiple delimiters

Address reviewer feedback - clarify that the test verifies behavior
for any non-empty delimiter, not just '/'.

* docs: clarify test comment with delimiter examples

Add specific examples of delimiters ('/', '_', ':') to make it clear
that the test verifies behavior with multiple delimiter types.

* fix: revert line 549 to original logic, only line 260 needed changing

The fix for directories being listed as objects only required changing
line 260 from 'delimiter == "/"' to 'delimiter != ""'.

Line 549 should remain as 'delimiter != "/"' to allow recursion for
delimiters that don't exist in paths (e.g., delimiter=z for paths like
b/a/c). This is correct S3 behavior.

Updated test to only verify delimiter="/" since other delimiters should
recurse into directories to find actual files.

* docs: clarify test scope in directory listing test
pull/7952/head
Chris Lu 1 week ago
committed by GitHub
parent
commit
f2373f9e8d
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 2
      weed/s3api/s3api_object_handlers_list.go
  2. 63
      weed/s3api/s3api_object_handlers_list_directory_test.go

2
weed/s3api/s3api_object_handlers_list.go

@ -257,7 +257,7 @@ func (s3a *S3ApiServer) listFilerEntries(bucket string, originalPrefix string, m
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.
} else if delimiter != "" { // A response can contain CommonPrefixes only if you specify a delimiter.
// Use raw dir and entry.Name (not encoded) to ensure consistent handling
// Encoding will be applied after sorting if encodingTypeUrl is set
commonPrefixes = append(commonPrefixes, PrefixEntry{

63
weed/s3api/s3api_object_handlers_list_directory_test.go

@ -0,0 +1,63 @@
package s3api
import (
"testing"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/stretchr/testify/assert"
)
// TestDirectoryListedAsCommonPrefix tests that regular directories (without MIME)
// are correctly listed as CommonPrefixes when delimiter="/" is used, not as objects.
// Note: The fix applies to any non-empty delimiter ('/', '_', ':'), but this test focuses on '/'.
func TestDirectoryListedAsCommonPrefix(t *testing.T) {
s3a := &S3ApiServer{
option: &S3ApiServerOption{
BucketsPath: "/buckets",
},
}
// Regular directory (no MIME) - matching actual user metadata
regularDir := &filer_pb.Entry{
Name: "f2f1237f-0e69-4e0b-8f01-d4fa299787e1.vhd",
IsDirectory: true,
Attributes: &filer_pb.FuseAttributes{
Mime: "", // Empty MIME - IsDirectoryKeyObject() returns false
},
}
client := &testFilerClient{
entriesByDir: map[string][]*filer_pb.Entry{
"/buckets/xoa-bucket/xo-vm-backups/data": {regularDir},
},
}
cursor := &ListingCursor{maxKeys: 1000}
var seenDirs []string
var seenFiles []string
_, err := s3a.doListFilerEntries(
client,
"/buckets/xoa-bucket/xo-vm-backups/data",
"", // prefix
cursor,
"", // marker
"/", // delimiter="/" - should yield directory for CommonPrefix processing
false,
"xoa-bucket",
func(dir string, entry *filer_pb.Entry) {
if entry.IsDirectory {
seenDirs = append(seenDirs, entry.Name)
} else {
seenFiles = append(seenFiles, entry.Name)
}
},
)
assert.NoError(t, err)
// The directory should be passed to the callback for delimiter="/"
assert.Contains(t, seenDirs, "f2f1237f-0e69-4e0b-8f01-d4fa299787e1.vhd",
"Directory should be passed to callback for CommonPrefix processing with delimiter=/")
assert.Empty(t, seenFiles, "No files should be seen, only the directory")
}
Loading…
Cancel
Save