s3: no empty-directory marker in a "<dir>/" self-prefix listing (#11906)

An out-of-band empty directory listed under its own trailing-slash
prefix answered the probe with a fake entry of itself: a Contents key on
4.48 and, since recently, a CommonPrefix. That listing enumerates
children, where S3 returns nothing for an empty prefix, and clients that
read every returned key (Pulumi lock listing) choked on the marker.

The prefix object and directory key object paths are unchanged: a real
"dir/" object still lists as its own key.

Generated with [Devin](https://devin.ai)

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

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 "<name>/" 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 "<dir>/"
-			// 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 "<dir>/" 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

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
-// "<dir>/" 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 "<dir>/" 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")
 }