s3: list nothing under an empty directory's own prefix

A trailing-slash prefix list enumerates the directory's children; AWS
answers empty when the directory holds no keys. The empty-directory
marker surfaced the directory itself -- as a Contents key first, as a
CommonPrefix since the delimited case was reworked -- so a client that
reads every returned key or prefix (Pulumi's lock listing, Terraform
state) treats the leftover directory as an object and fails on it.

Drop the marker: the probe now returns nothing, matching AWS. Directory
key objects written by PutObject "dir/" still list as keys, and a
non-empty directory is still represented by its children.

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,25 +1009,15 @@ 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
-			if cursor.prefixEndsOnDelimiter {
-				cursor.prefixEndsOnDelimiter = false
-			}
+			// A trailing-slash prefix names one directory; walk only it, then stop.
+			cursor.prefixEndsOnDelimiter = false
 			if isKeyObject && blockKey > marker {
 				// Directory key objects (created via PutObject with trailing "/")
 				// 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)
-			})
+			// Recurse into subdirectory to list any children.
+			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 +1027,6 @@ 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)
-			}
 			// 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) {
+// TestEmptyDirectoryListsNothingUnderOwnPrefix pins the listing of an empty
+// directory (created via mount, mkdir or the filer API, or left behind by
+// deleted objects) under its own "<dir>/" prefix: both the recursive and the
+// delimited probe answer empty, matching AWS S3.
+func TestEmptyDirectoryListsNothingUnderOwnPrefix(t *testing.T) {
 	s3a := &S3ApiServer{
 		option: &S3ApiServerOption{BucketsPath: "/buckets"},
 	}
@@ -87,20 +86,16 @@ 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)
-		})
+	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)
-	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")
+		assert.NoError(t, err)
+		assert.Empty(t, seen, "an empty directory's own prefix must list nothing (delimiter=%q)", delimiter)
 	}
 }
 
@@ -185,10 +180,11 @@ 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 delimited listing of an empty directory's own prefix enumerates children,
+// so it must answer empty like AWS S3: no Contents key and no CommonPrefix
+// equal to the request prefix. Clients that treat every returned key as an
+// object (Pulumi's lock listing, for one) must not see the leftover directory.
+func TestEmptyDirectoryDelimitedListingIsEmpty(t *testing.T) {
 	filer := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{
 		"/buckets/test": {
 			{Name: "locks", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Mime: ""}},
@@ -208,6 +204,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 names no prefix of its own")
 }