diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 3cc7b1e49..23db5c6ee 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -539,6 +539,18 @@ 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 != "" { @@ -789,6 +801,9 @@ 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 } // excludedMarkerKey returns the key an exclusive marker names that the walk's cutoff no @@ -1124,15 +1139,21 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb. // 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. Surface it as a directory marker for that explicit probe, - // identical to a directory created via PutObject with a trailing "/", so - // tools like hadoop-aws can find it. Plain listings are left untouched, so - // empty directories left behind by deleted objects are not shown as keys. + // 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 && entry.Attributes != nil && !s3a.dirHoldsOnlyHiddenEntries(ctx, client, bucket, dir+"/"+entry.Name, cursor) { - entry.Attributes.Mime = s3_constants.FolderMimeType + if delimiter == "/" { + cursor.emitDirAsCommonPrefix = true + } else { + entry.Attributes.Mime = s3_constants.FolderMimeType + } eachEntryFn(dir, entry) } // println("doListFilerEntries2 dir", dir+"/"+entry.Name, "subNextMarker", subNextMarker) diff --git a/weed/s3api/s3api_object_handlers_list_directory_test.go b/weed/s3api/s3api_object_handlers_list_directory_test.go index e4e65500a..ba0413b8f 100644 --- a/weed/s3api/s3api_object_handlers_list_directory_test.go +++ b/weed/s3api/s3api_object_handlers_list_directory_test.go @@ -3,10 +3,11 @@ package s3api import ( "context" "testing" + "time" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" - "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestDirectoryListedAsCommonPrefix tests that regular directories (without MIME) @@ -67,8 +68,8 @@ func TestDirectoryListedAsCommonPrefix(t *testing.T) { // 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. Such a directory is surfaced as a directory marker, identical to -// one created via PutObject with a trailing "/". +// "/" 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) { s3a := &S3ApiServer{ option: &S3ApiServerOption{BucketsPath: "/buckets"}, @@ -96,9 +97,10 @@ func TestEmptyDirectorySurfacedAsMarker(t *testing.T) { assert.NoError(t, err) if assert.Len(t, seen, 1, "empty directory must be surfaced under the logs/ prefix") { - assert.True(t, seen[0].IsDirectoryKeyObject(), - "empty directory must be marked as a directory key object so it lists as logs/") - assert.Equal(t, s3_constants.FolderMimeType, seen[0].Attributes.Mime) + 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") } } @@ -182,3 +184,30 @@ func TestEmptyDirectoryHiddenInFlatListing(t *testing.T) { assert.NoError(t, err) 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) { + filer := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/test": { + {Name: "locks", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Mime: ""}}, + }, + "/buckets/test/locks": {}, + }} + s3a := newFailoverTestServer(t, startFakeFiler(t, filer)) + s3a.option.BucketsPath = "/buckets" + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set("test", &BucketConfig{Name: "test"}) + + response, err := s3a.listFilerEntries(context.Background(), listObjectsRequest{ + bucket: "test", + prefix: "locks/", + delimiter: "/", + maxKeys: 1000, + }) + 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) +}