From f2373f9e8d6ead42521d730afbca2fd22e351d67 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 2 Jan 2026 15:52:37 -0800 Subject: [PATCH] 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 --- weed/s3api/s3api_object_handlers_list.go | 2 +- ...api_object_handlers_list_directory_test.go | 63 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 weed/s3api/s3api_object_handlers_list_directory_test.go diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 0aecc8655..0f9b5a93a 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/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{ diff --git a/weed/s3api/s3api_object_handlers_list_directory_test.go b/weed/s3api/s3api_object_handlers_list_directory_test.go new file mode 100644 index 000000000..8b3c26b8b --- /dev/null +++ b/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") +}