From 8491001d1104ab7c49c8892140cdf2d7d18a21d3 Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 21 Mar 2026 14:07:39 -0700 Subject: [PATCH] fix(s3): list directory markers with explicit Content-Type --- .../s3/normal/s3_list_empty_directory_test.go | 110 ++++++++++++++++++ .../empty_folder_cleaner.go | 46 +++++++- weed/pb/filer_pb/filer_pb_helper.go | 13 ++- weed/s3api/s3_constants/extend_key.go | 1 + weed/s3api/s3api_object_handlers_list_test.go | 71 +++++++++++ weed/s3api/s3api_object_handlers_put.go | 25 +++- 6 files changed, 257 insertions(+), 9 deletions(-) diff --git a/test/s3/normal/s3_list_empty_directory_test.go b/test/s3/normal/s3_list_empty_directory_test.go index b8c64180f..f10f563b8 100644 --- a/test/s3/normal/s3_list_empty_directory_test.go +++ b/test/s3/normal/s3_list_empty_directory_test.go @@ -223,6 +223,116 @@ func TestS3ListObjectsEmptyDirectoryMarkers(t *testing.T) { }) } +// TestS3ListObjectsDirectoryMarkerWithContentType reproduces GitHub issue #8712: +// directory markers created with an explicit Content-Type must still appear in listings. +func TestS3ListObjectsDirectoryMarkerWithContentType(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + cluster, err := startMiniCluster(t) + require.NoError(t, err) + defer cluster.Stop() + + bucketName := createTestBucket(t, cluster, "test-content-type-dirs-") + + _, err = cluster.s3Client.PutObject(&s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String("test-content/empty/"), + Body: bytes.NewReader([]byte{}), + ContentType: aws.String("application/octet-stream"), + }) + require.NoError(t, err, "failed to create directory marker with content-type") + + headResp, err := cluster.s3Client.HeadObject(&s3.HeadObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String("test-content/empty/"), + }) + require.NoError(t, err, "directory marker should exist via HeadObject") + assert.Equal(t, "application/octet-stream", aws.StringValue(headResp.ContentType)) + + t.Run("ListV2_WithPrefix_NoDelimiter", func(t *testing.T) { + resp, err := cluster.s3Client.ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: aws.String(bucketName), + Prefix: aws.String("test-content"), + }) + require.NoError(t, err) + + keys := collectKeys(resp.Contents) + assert.Equal(t, []string{"test-content/empty/"}, keys) + require.Equal(t, 1, len(resp.Contents)) + assert.Equal(t, int64(0), aws.Int64Value(resp.Contents[0].Size)) + }) + + t.Run("ListV1_WithPrefix_NoDelimiter", func(t *testing.T) { + resp, err := cluster.s3Client.ListObjects(&s3.ListObjectsInput{ + Bucket: aws.String(bucketName), + Prefix: aws.String("test-content"), + }) + require.NoError(t, err) + + keys := collectKeysV1(resp.Contents) + assert.Equal(t, []string{"test-content/empty/"}, keys) + require.Equal(t, 1, len(resp.Contents)) + assert.Equal(t, int64(0), aws.Int64Value(resp.Contents[0].Size)) + }) + + t.Run("ListV2_NoPrefix", func(t *testing.T) { + resp, err := cluster.s3Client.ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: aws.String(bucketName), + }) + require.NoError(t, err) + + keys := collectKeys(resp.Contents) + assert.Contains(t, keys, "test-content/empty/") + + var dirMarker *s3.Object + for _, obj := range resp.Contents { + if aws.StringValue(obj.Key) == "test-content/empty/" { + dirMarker = obj + break + } + } + require.NotNil(t, dirMarker) + assert.Equal(t, int64(0), aws.Int64Value(dirMarker.Size)) + }) + + t.Run("ListV2_WithDelimiter", func(t *testing.T) { + resp, err := cluster.s3Client.ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: aws.String(bucketName), + Delimiter: aws.String("/"), + }) + require.NoError(t, err) + + prefixes := collectPrefixes(resp.CommonPrefixes) + assert.Contains(t, prefixes, "test-content/") + }) + + t.Run("ListV2_MixedMimeTypes", func(t *testing.T) { + _, err := cluster.s3Client.PutObject(&s3.PutObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String("test-content/default/"), + Body: bytes.NewReader([]byte{}), + }) + require.NoError(t, err) + + resp, err := cluster.s3Client.ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: aws.String(bucketName), + Prefix: aws.String("test-content"), + }) + require.NoError(t, err) + + keys := collectKeys(resp.Contents) + sort.Strings(keys) + assert.Equal(t, []string{"test-content/default/", "test-content/empty/"}, keys) + + require.Equal(t, 2, len(resp.Contents)) + for _, obj := range resp.Contents { + assert.Equal(t, int64(0), aws.Int64Value(obj.Size)) + } + }) +} + func collectKeys(contents []*s3.Object) []string { keys := make([]string, 0, len(contents)) for _, obj := range contents { diff --git a/weed/filer/empty_folder_cleanup/empty_folder_cleaner.go b/weed/filer/empty_folder_cleanup/empty_folder_cleaner.go index 943d86c6f..5401537d8 100644 --- a/weed/filer/empty_folder_cleanup/empty_folder_cleaner.go +++ b/weed/filer/empty_folder_cleanup/empty_folder_cleaner.go @@ -2,6 +2,7 @@ package empty_folder_cleanup import ( "context" + "errors" "strings" "sync" "time" @@ -31,10 +32,11 @@ type FilerOperations interface { // folderState tracks the state of a folder for empty folder cleanup type folderState struct { - roughCount int // Cached rough count (up to maxCountCheck) - lastAddTime time.Time // Last time an item was added - lastDelTime time.Time // Last time an item was deleted - lastCheck time.Time // Last time we checked the actual count + roughCount int // Cached rough count (up to maxCountCheck) + lastAddTime time.Time // Last time an item was added + lastDelTime time.Time // Last time an item was deleted + lastCheck time.Time // Last time we checked the actual count + isDirectoryMarker *bool // nil means not checked yet } type bucketCleanupPolicyState struct { @@ -312,6 +314,26 @@ func (efc *EmptyFolderCleaner) executeCleanup(folder string, triggeredBy string) return } + efc.mu.Lock() + state := efc.folderCounts[folder] + var isMarker bool + if state != nil && state.isDirectoryMarker != nil { + isMarker = *state.isDirectoryMarker + } else { + efc.mu.Unlock() + isMarker = efc.isDirectoryMarker(ctx, folder) + efc.mu.Lock() + if state, exists := efc.folderCounts[folder]; exists && state != nil { + state.isDirectoryMarker = &isMarker + } + } + efc.mu.Unlock() + + if isMarker { + glog.V(3).Infof("EmptyFolderCleaner: skipping deletion of directory marker %s (triggered by %s)", folder, triggeredBy) + return + } + // Delete the empty folder glog.Infof("EmptyFolderCleaner: deleting empty folder %s (triggered by %s)", folder, triggeredBy) if err := efc.deleteFolder(ctx, folder); err != nil { @@ -339,6 +361,22 @@ func (efc *EmptyFolderCleaner) deleteFolder(ctx context.Context, folder string) return efc.filer.DeleteEntryMetaAndData(ctx, util.FullPath(folder), false, false, false, false, nil, 0) } +func (efc *EmptyFolderCleaner) isDirectoryMarker(ctx context.Context, folder string) bool { + attrs, err := efc.filer.GetEntryAttributes(ctx, util.FullPath(folder)) + if err != nil { + if errors.Is(err, filer_pb.ErrNotFound) { + return false + } + glog.V(2).Infof("EmptyFolderCleaner: error reading attributes for %s, skipping deletion: %v", folder, err) + return true + } + if attrs == nil { + return false + } + _, hasMime := attrs[s3_constants.ExtMimeType] + return hasMime +} + func (efc *EmptyFolderCleaner) getBucketCleanupPolicy(ctx context.Context, folder string) (bucketPath string, autoRemove bool, source string, attrValue string, err error) { bucketPath, ok := util.ExtractBucketPath(efc.bucketPath, folder, true) if !ok { diff --git a/weed/pb/filer_pb/filer_pb_helper.go b/weed/pb/filer_pb/filer_pb_helper.go index fed902824..aa3da8270 100644 --- a/weed/pb/filer_pb/filer_pb_helper.go +++ b/weed/pb/filer_pb/filer_pb_helper.go @@ -22,7 +22,18 @@ func (entry *Entry) IsInRemoteOnly() bool { } func (entry *Entry) IsDirectoryKeyObject() bool { - return entry.IsDirectory && entry.Attributes != nil && entry.Attributes.Mime != "" + if !entry.IsDirectory || entry.Attributes == nil { + return false + } + if entry.Attributes.Mime != "" { + return true + } + if entry.Extended != nil { + if _, hasMime := entry.Extended[s3_constants.ExtMimeType]; hasMime { + return true + } + } + return false } func (entry *Entry) GetExpiryTime() (expiryTime int64) { diff --git a/weed/s3api/s3_constants/extend_key.go b/weed/s3api/s3_constants/extend_key.go index 8e5aeade5..0ca703dfa 100644 --- a/weed/s3api/s3_constants/extend_key.go +++ b/weed/s3api/s3_constants/extend_key.go @@ -19,6 +19,7 @@ const ( ExtLatestVersionOwnerKey = "Seaweed-X-Amz-Latest-Version-Owner" ExtLatestVersionIsDeleteMarker = "Seaweed-X-Amz-Latest-Version-Is-Delete-Marker" ExtMultipartObjectKey = "key" + ExtMimeType = "Seaweed-X-Amz-Mime-Type" // Bucket Policy ExtBucketPolicyKey = "Seaweed-X-Amz-Bucket-Policy" diff --git a/weed/s3api/s3api_object_handlers_list_test.go b/weed/s3api/s3api_object_handlers_list_test.go index ffbe81c80..911d56156 100644 --- a/weed/s3api/s3api_object_handlers_list_test.go +++ b/weed/s3api/s3api_object_handlers_list_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/stretchr/testify/assert" grpc "google.golang.org/grpc" @@ -284,6 +285,76 @@ func TestAllowUnorderedParameterValidation(t *testing.T) { }) } +func TestDoListFilerEntries_DirectoryKeyObjectWithCustomMimeType(t *testing.T) { + s3a := &S3ApiServer{ + option: &S3ApiServerOption{ + BucketsPath: "/buckets", + }, + } + + tests := []struct { + name string + entry *filer_pb.Entry + }{ + { + name: "default folder mime", + entry: &filer_pb.Entry{ + Name: "empty", + IsDirectory: true, + Attributes: &filer_pb.FuseAttributes{Mime: s3_constants.FolderMimeType}, + }, + }, + { + name: "application octet stream mime", + entry: &filer_pb.Entry{ + Name: "empty", + IsDirectory: true, + Attributes: &filer_pb.FuseAttributes{Mime: "application/octet-stream"}, + }, + }, + { + name: "custom directory mime", + entry: &filer_pb.Entry{ + Name: "empty", + IsDirectory: true, + Attributes: &filer_pb.FuseAttributes{Mime: "application/x-directory"}, + }, + }, + { + name: "mime stripped but extended marker present", + entry: &filer_pb.Entry{ + Name: "empty", + IsDirectory: true, + Attributes: &filer_pb.FuseAttributes{}, + Extended: map[string][]byte{s3_constants.ExtMimeType: []byte("application/octet-stream")}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &testFilerClient{ + entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/bucket": { + {Name: "test-content", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}}, + }, + "/buckets/bucket/test-content": {tt.entry}, + "/buckets/bucket/test-content/empty": {}, + }, + } + + cursor := &ListingCursor{maxKeys: 1000} + var seen []string + _, err := s3a.doListFilerEntries(client, "/buckets/bucket", "test-content", cursor, "", "", false, "bucket", func(dir string, entry *filer_pb.Entry) { + seen = append(seen, entry.Name) + }) + + assert.NoError(t, err) + assert.Contains(t, seen, "empty") + }) + } +} + func TestDoListFilerEntries_BucketRootPrefixSlashDelimiterSlash_ListsDirectories(t *testing.T) { // Regression test for a bug where doListFilerEntries returned early when // prefix == "/" && delimiter == "/", causing bucket-root folder listings diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index d944f496c..989b6b45e 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -122,7 +122,14 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) defer dataReader.Close() objectContentType := r.Header.Get("Content-Type") - if strings.HasSuffix(object, "/") && r.ContentLength <= 1024 { + actualContentLength := r.ContentLength + if decodedStr := r.Header.Get("X-Amz-Decoded-Content-Length"); decodedStr != "" { + if decoded, parseErr := strconv.ParseInt(decodedStr, 10, 64); parseErr == nil && decoded >= 0 { + actualContentLength = decoded + } + } + + if strings.HasSuffix(object, "/") && actualContentLength >= 0 && actualContentLength <= 1024 { // Split the object into directory path and name objectWithoutSlash := strings.TrimSuffix(object, "/") dirName := path.Dir(objectWithoutSlash) @@ -139,16 +146,23 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) fullDirPath = fullDirPath + "/" + dirName } - // Read any content through dataReader (handles chunked encoding properly) + // Read any content through dataReader (handles chunked encoding properly). + // Use the decoded content length when AWS chunked encoding is active. var dirContent []byte - if r.ContentLength != 0 { + if actualContentLength != 0 { var readErr error - dirContent, readErr = io.ReadAll(dataReader) + limitedReader := io.LimitReader(dataReader, 1024+1) + dirContent, readErr = io.ReadAll(limitedReader) if readErr != nil { glog.Errorf("PutObjectHandler: failed to read directory marker content %s/%s: %v", bucket, object, readErr) s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) return } + if len(dirContent) > 1024 { + glog.Warningf("PutObjectHandler: directory marker payload exceeds 1024 bytes: %s/%s (size=%d)", bucket, object, len(dirContent)) + s3err.WriteErrorResponse(w, r, s3err.ErrEntityTooLarge) + return + } } // Compute MD5 for ETag (md5.Sum of nil/empty = MD5 of empty content) @@ -174,6 +188,9 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) entry.Extended = make(map[string][]byte) } entry.Extended[s3_constants.ExtETagKey] = []byte(dirEtag) + if len(dirContent) == 0 { + entry.Extended[s3_constants.ExtMimeType] = []byte(objectContentType) + } // Set object owner for directory objects (same as regular objects) s3a.setObjectOwnerFromRequest(r, bucket, entry)