diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index b6df5ea562..679c9f438a 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -540,18 +540,6 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq } dirName, entryName, _ := entryUrlEncode(dir, entry.Name, encodingTypeUrl) if entry.IsDirectory { - if cursor.emitDirAsCommonPrefix { - cursor.emitDirAsCommonPrefix = false - dirPrefix := fmt.Sprintf("%s/%s/", dir, entry.Name)[len(bucketPrefix):] - commonPrefixes = append(commonPrefixes, PrefixEntry{ - Prefix: dirPrefix, - }) - cursor.maxKeys-- - lastEntryWasCommonPrefix = true - lastCommonPrefix = dirPrefix - lastPrefixNullBackers, lastPrefixConfirmed = nil, true - return - } if originalPrefix != "" { normalizedPrefix := strings.TrimPrefix(strings.TrimSuffix(originalPrefix, "/"), "/") if normalizedPrefix != "" { @@ -806,9 +794,6 @@ type ListingCursor struct { // resolvePendingNulls settles trailing null objects whose .versions sibling // has not streamed yet before a page is declared full. resolvePendingNulls func() error - // emitDirAsCommonPrefix marks the next directory entry passed to the entry - // callback for reporting as a CommonPrefix rather than a Contents key. - emitDirAsCommonPrefix bool // prefixObjectKeyOnly limits the next prefix-object entry passed to the entry // callback to its plain key; its "/" CommonPrefix is deferred to the // directory's own block position. @@ -1024,10 +1009,8 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb. } isKeyObject := e.IsDirectoryKeyObject() && !e.IsPrefixObject() if delimiter != "/" || cursor.prefixEndsOnDelimiter { - // A trailing-slash prefix (e.g. "logs/") names one directory and asks - // whether it exists, so a real but empty directory must be reported for - // that probe. - explicitDirProbe := cursor.prefixEndsOnDelimiter + // A trailing-slash prefix (e.g. "logs/") names one directory, so the + // recursion below answers whether it exists and what it holds. if cursor.prefixEndsOnDelimiter { cursor.prefixEndsOnDelimiter = false } @@ -1036,13 +1019,7 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb. // appear as regular keys in recursive listing mode. eachEntryFn(dir, emitEntry) } - // Recurse into subdirectory to list any children, noting whether the - // subtree produced any entries. - childEmitted := false - subNextMarker, subErr := s3a.doListFilerEntries(ctx, client, listDirectoryRequest{dir: dir + "/" + e.Name, marker: subMarker, delimiter: delimiter, bucket: bucket}, cursor, func(d string, ent *filer_pb.Entry) { - childEmitted = true - eachEntryFn(d, ent) - }) + subNextMarker, subErr := s3a.doListFilerEntries(ctx, client, listDirectoryRequest{dir: dir + "/" + e.Name, marker: subMarker, delimiter: delimiter, bucket: bucket}, cursor, eachEntryFn) if subErr != nil { return fmt.Errorf("doListFilerEntries2: %w", subErr) } @@ -1052,26 +1029,10 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb. // keep a resume position that re-enters it. cursor.isTruncated = true } - // A real but empty directory (created out of band via mount, mkdir or - // the filer API, so it carries no MIME) is otherwise invisible to S3 - // clients that detect directories by listing it under its own "/" - // prefix. Answer that explicit probe so tools like hadoop-aws can find - // the directory: a delimited listing reports it as a CommonPrefix -- - // a real key in Contents would look like an object a client can read -- - // while a recursive listing keeps the directory marker key it used. - // Plain listings are left untouched, so empty directories left behind - // by deleted objects are not shown as keys. - // A directory that still holds version history no longer names anything, - // so it gets no marker either. - if explicitDirProbe && !isKeyObject && !childEmitted && !cursor.isTruncated && e.Attributes != nil && - !s3a.dirHoldsOnlyHiddenEntries(ctx, client, bucket, dir+"/"+e.Name, cursor) { - if delimiter == "/" { - cursor.emitDirAsCommonPrefix = true - } else { - emitEntry.Attributes.Mime = s3_constants.FolderMimeType - } - eachEntryFn(dir, emitEntry) - } + // An empty directory gets no marker under its own "/" prefix: + // that listing enumerates children, where S3 returns nothing for an + // empty prefix, and a fake entry is read back as a real object by + // clients that enumerate locks or similar. // println("doListFilerEntries2 dir", dir+"/"+e.Name, "subNextMarker", subNextMarker) if cursor.isTruncated || matchedPrefixDir { stopped = true diff --git a/weed/s3api/s3api_object_handlers_list_directory_test.go b/weed/s3api/s3api_object_handlers_list_directory_test.go index ba0413b8f5..d74a75d980 100644 --- a/weed/s3api/s3api_object_handlers_list_directory_test.go +++ b/weed/s3api/s3api_object_handlers_list_directory_test.go @@ -65,12 +65,11 @@ func TestDirectoryListedAsCommonPrefix(t *testing.T) { assert.Empty(t, seenFiles, "No files should be seen, only the directory") } -// TestEmptyDirectorySurfacedAsMarker reproduces the hadoop-aws / Spark case where a -// real but empty directory (created via mount, mkdir or the filer API, so it has no -// MIME) must be visible to S3 clients that detect directories by listing under the -// "/" prefix. Under a delimiter it is reported as a CommonPrefix, which the -// probe accepts, instead of a key a client might try to read. -func TestEmptyDirectorySurfacedAsMarker(t *testing.T) { +// TestEmptyDirectoryNotListedUnderOwnPrefix covers the explicit "/" probe +// on a real but empty directory (created via mount, mkdir or the filer API, so +// it has no MIME): it is listed under its own prefix in neither delimited nor +// recursive mode, matching S3 where an empty prefix has no key to return. +func TestEmptyDirectoryNotListedUnderOwnPrefix(t *testing.T) { s3a := &S3ApiServer{ option: &S3ApiServerOption{BucketsPath: "/buckets"}, } @@ -87,20 +86,15 @@ func TestEmptyDirectorySurfacedAsMarker(t *testing.T) { }, } - // Mirrors the getFileStatus probe: prefix "logs/" with delimiter "/". - cursor := &ListingCursor{maxKeys: 1000, prefixEndsOnDelimiter: true} - var seen []*filer_pb.Entry - _, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{dir: "/buckets/test", prefix: "logs", delimiter: "/", bucket: "test"}, cursor, - func(dir string, entry *filer_pb.Entry) { - seen = append(seen, entry) - }) - - assert.NoError(t, err) - if assert.Len(t, seen, 1, "empty directory must be surfaced under the logs/ prefix") { - assert.True(t, cursor.emitDirAsCommonPrefix, - "a delimited probe reports the empty directory as a common prefix") - assert.Empty(t, seen[0].Attributes.Mime, - "the entry must not be dressed as a directory key object") + for _, delimiter := range []string{"/", ""} { + cursor := &ListingCursor{maxKeys: 1000, prefixEndsOnDelimiter: true} + var seen []*filer_pb.Entry + _, err := s3a.doListFilerEntries(context.Background(), client, listDirectoryRequest{dir: "/buckets/test", prefix: "logs", delimiter: delimiter, bucket: "test"}, cursor, + func(dir string, entry *filer_pb.Entry) { + seen = append(seen, entry) + }) + assert.NoError(t, err) + assert.Empty(t, seen, "an empty directory reports nothing under its own prefix (delimiter %q)", delimiter) } } @@ -185,10 +179,10 @@ func TestEmptyDirectoryHiddenInFlatListing(t *testing.T) { assert.Equal(t, []string{"foo"}, seen, "flat listing must hide the empty directory and return only real objects") } -// A trailing-slash delimited listing of an empty directory reports the -// directory as a CommonPrefix, not as a Contents key: clients that treat -// every returned key as an object (e.g. lock listings) skip prefixes. -func TestEmptyDirectoryListedAsCommonPrefix(t *testing.T) { +// A trailing-slash delimited listing of an empty directory enumerates its +// children, so it returns nothing: the directory is neither a readable Contents +// key nor a CommonPrefix of its own. +func TestEmptyDirectoryHiddenInDelimitedSelfListing(t *testing.T) { filer := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{ "/buckets/test": { {Name: "locks", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Mime: ""}}, @@ -208,6 +202,5 @@ func TestEmptyDirectoryListedAsCommonPrefix(t *testing.T) { }) require.NoError(t, err) assert.Empty(t, response.Contents, "an empty directory is not a readable key") - require.Len(t, response.CommonPrefixes, 1) - assert.Equal(t, "locks/", response.CommonPrefixes[0].Prefix) + assert.Empty(t, response.CommonPrefixes, "an empty directory has no prefixes to fold into") }