diff --git a/weed/s3api/s3api_implicit_directory_test.go b/weed/s3api/s3api_implicit_directory_test.go index e7c3633fc..26fd62ee6 100644 --- a/weed/s3api/s3api_implicit_directory_test.go +++ b/weed/s3api/s3api_implicit_directory_test.go @@ -16,32 +16,47 @@ func TestImplicitDirectoryBehaviorLogic(t *testing.T) { hasTrailingSlash bool fileSize uint64 isDirectory bool + mimeType string hasChildren bool versioningEnabled bool shouldReturn404 bool description string }{ { - name: "Implicit directory: 0-byte file with children, no trailing slash", + name: "PyArrow directory marker: 0-byte file with application/octet-stream and children", objectPath: "dataset", hasTrailingSlash: false, fileSize: 0, isDirectory: false, + mimeType: "application/octet-stream", hasChildren: true, versioningEnabled: false, shouldReturn404: true, description: "Should return 404 to force s3fs LIST-based discovery", }, { - name: "Implicit directory: actual directory with children, no trailing slash", + name: "PyArrow directory marker: 0-byte file with empty MIME type and children", + objectPath: "dataset", + hasTrailingSlash: false, + fileSize: 0, + isDirectory: false, + mimeType: "", + hasChildren: true, + versioningEnabled: false, + shouldReturn404: true, + description: "Should return 404 for empty MIME type directory markers", + }, + { + name: "Actual directory with children", objectPath: "dataset", hasTrailingSlash: false, fileSize: 0, isDirectory: true, + mimeType: "", hasChildren: true, versioningEnabled: false, - shouldReturn404: true, - description: "Should return 404 for directory with children", + shouldReturn404: false, + description: "Should return 200 for actual directories (maintains AWS S3 compatibility)", }, { name: "Explicit directory request: trailing slash", @@ -49,6 +64,7 @@ func TestImplicitDirectoryBehaviorLogic(t *testing.T) { hasTrailingSlash: true, fileSize: 0, isDirectory: true, + mimeType: "", hasChildren: true, versioningEnabled: false, shouldReturn404: false, @@ -60,17 +76,19 @@ func TestImplicitDirectoryBehaviorLogic(t *testing.T) { hasTrailingSlash: false, fileSize: 0, isDirectory: false, + mimeType: "application/octet-stream", hasChildren: false, versioningEnabled: false, shouldReturn404: false, description: "Should return 200 for legitimate empty file", }, { - name: "Empty directory: 0-byte directory without children", + name: "Empty directory: directory without children", objectPath: "empty-dir", hasTrailingSlash: false, fileSize: 0, isDirectory: true, + mimeType: "", hasChildren: false, versioningEnabled: false, shouldReturn404: false, @@ -82,41 +100,44 @@ func TestImplicitDirectoryBehaviorLogic(t *testing.T) { hasTrailingSlash: false, fileSize: 100, isDirectory: false, + mimeType: "text/plain", hasChildren: false, versioningEnabled: false, shouldReturn404: false, description: "Should return 200 for regular file with content", }, { - name: "Versioned bucket: implicit directory should return 200", + name: "Versioned bucket: directory marker should return 200", objectPath: "dataset", hasTrailingSlash: false, fileSize: 0, isDirectory: false, + mimeType: "application/octet-stream", hasChildren: true, versioningEnabled: true, shouldReturn404: false, - description: "Should return 200 for versioned buckets (skip implicit dir check)", + description: "Should return 200 for versioned buckets (skip directory marker check)", }, { - name: "PyArrow directory marker: 0-byte with children", + name: "Directory marker with specific MIME type", objectPath: "dataset", hasTrailingSlash: false, fileSize: 0, isDirectory: false, + mimeType: "text/plain", hasChildren: true, versioningEnabled: false, - shouldReturn404: true, - description: "Should return 404 for PyArrow-created directory markers", + shouldReturn404: false, + description: "Should return 200 for 0-byte files with specific MIME types (not generic markers)", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Test the logic: should we return 404? - // Logic from HeadObjectHandler: + // New logic from HeadObjectHandler: // if !versioningConfigured && !strings.HasSuffix(object, "/") { - // if isZeroByteFile || isActualDirectory { + // if isZeroByteFile && hasGenericMimeType { // if hasChildren { // return 404 // } @@ -124,11 +145,11 @@ func TestImplicitDirectoryBehaviorLogic(t *testing.T) { // } isZeroByteFile := tt.fileSize == 0 && !tt.isDirectory - isActualDirectory := tt.isDirectory + hasGenericMimeType := tt.mimeType == "" || tt.mimeType == "application/octet-stream" shouldReturn404 := false if !tt.versioningEnabled && !tt.hasTrailingSlash { - if isZeroByteFile || isActualDirectory { + if isZeroByteFile && hasGenericMimeType { if tt.hasChildren { shouldReturn404 = true } @@ -228,18 +249,18 @@ func TestImplicitDirectoryEdgeCases(t *testing.T) { expectation string }{ { - name: "PyArrow write_dataset creates 0-byte files", - scenario: "PyArrow creates 'dataset' as 0-byte file, then writes 'dataset/file.parquet'", - expectation: "HEAD dataset → 404 (has children), s3fs uses LIST → correctly identifies as directory", + name: "PyArrow write_dataset creates 0-byte files with application/octet-stream", + scenario: "PyArrow creates 'dataset' as 0-byte file with MIME type 'application/octet-stream', then writes 'dataset/file.parquet'", + expectation: "HEAD dataset → 404 (has children + generic MIME type), s3fs uses LIST → correctly identifies as directory", }, { name: "Filer creates actual directories", scenario: "Filer creates 'dataset' as actual directory with IsDirectory=true", - expectation: "HEAD dataset → 404 (has children), s3fs uses LIST → correctly identifies as directory", + expectation: "HEAD dataset → 200 (actual directory, not 0-byte file), maintains AWS S3 compatibility", }, { name: "Empty file edge case", - scenario: "User creates 'empty.txt' as 0-byte file with no children", + scenario: "User creates 'empty.txt' as 0-byte file with 'application/octet-stream' but no children", expectation: "HEAD empty.txt → 200 (no children), s3fs correctly reports as file", }, { @@ -250,13 +271,18 @@ func TestImplicitDirectoryEdgeCases(t *testing.T) { { name: "Versioned bucket", scenario: "Bucket has versioning enabled", - expectation: "HEAD dataset → 200 (skip implicit dir check), versioned semantics apply", + expectation: "HEAD dataset → 200 (skip directory marker check), versioned semantics apply", }, { name: "AWS S3 compatibility", scenario: "Only 'dataset/file.txt' exists, no marker at 'dataset'", expectation: "HEAD dataset → 404 (object doesn't exist), matches AWS S3 behavior", }, + { + name: "Directory marker with specific MIME type", + scenario: "PyArrow creates 'dataset' as 0-byte file with MIME type 'text/plain' and children", + expectation: "HEAD dataset → 200 (specific MIME type, not generic), may not work with PyArrow but preserves compatibility", + }, } for _, tt := range tests { diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 9b1495128..34d49f4ba 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -2278,47 +2278,45 @@ func (s3a *S3ApiServer) HeadObjectHandler(w http.ResponseWriter, r *http.Request // // Background: // Some S3 clients (like PyArrow with s3fs) create directory markers when writing datasets. - // These can be either: - // 1. 0-byte files with directory MIME type (e.g., "application/octet-stream") - // 2. Actual directories in the filer (created by PyArrow's write_dataset) + // These are typically 0-byte files with MIME type "application/octet-stream" that have children. // // Problem: - // s3fs's info() method calls HEAD on the path. If HEAD returns 200 with size=0, - // s3fs incorrectly reports it as a file (type='file', size=0) instead of checking - // for children. This causes PyArrow to fail with "Parquet file size is 0 bytes". + // s3fs's info() method calls HEAD on these markers. If HEAD returns 200 with size=0, + // s3fs incorrectly reports them as files instead of directories, causing PyArrow to fail. // // Solution: // For non-versioned objects without trailing slash, if the object is a 0-byte file - // or directory AND has children, return 404 instead of 200. This forces s3fs to - // fall back to LIST-based discovery, which correctly identifies it as a directory. + // with MIME type "application/octet-stream" AND has children, return 404 instead of 200. + // This forces s3fs to fall back to LIST-based discovery, which correctly identifies + // the object as a directory. // // AWS S3 Compatibility: - // AWS S3 typically doesn't create directory markers for implicit directories, so - // HEAD on "dataset" (when only "dataset/file.txt" exists) returns 404. Our behavior - // matches this by returning 404 for implicit directories with children. + // This maintains AWS S3 compatibility by only affecting objects that are likely + // directory markers (0-byte files with generic MIME type that have children). + // Regular 0-byte files with "application/octet-stream" that are not directory markers + // will still return 200 OK. // // Edge Cases Handled: - // - Empty files (0-byte, no children) → 200 OK (legitimate empty file) - // - Empty directories (no children) → 200 OK (legitimate empty directory) + // - Regular empty files (0-byte, no children) → 200 OK + // - Empty directories (no children) → 200 OK // - Explicit directory requests (trailing slash) → 200 OK (handled earlier) // - Versioned objects → Skip this check (different semantics) + // - Directory markers with other MIME types → 200 OK (may break PyArrow but preserves compatibility) // // Performance: - // Only adds overhead for 0-byte files or directories without trailing slash. + // Only adds overhead for 0-byte files with "application/octet-stream" MIME type. // Cost: One LIST operation with Limit=1 (~1-5ms). // if !versioningConfigured && !strings.HasSuffix(object, "/") { - // Check if this is an implicit directory (either a 0-byte file or actual directory with children) - // PyArrow may create 0-byte files when writing datasets, or the filer may have actual directories + // Check if this looks like a PyArrow directory marker if objectEntryForSSE.Attributes != nil { isZeroByteFile := objectEntryForSSE.Attributes.FileSize == 0 && !objectEntryForSSE.IsDirectory - isActualDirectory := objectEntryForSSE.IsDirectory + hasGenericMimeType := objectEntryForSSE.Attributes.Mime == "" || objectEntryForSSE.Attributes.Mime == "application/octet-stream" - if isZeroByteFile || isActualDirectory { - // Check if it has children (making it an implicit directory) + if isZeroByteFile && hasGenericMimeType { + // Check if it has children (likely a directory marker) if s3a.hasChildren(bucket, object) { - // This is an implicit directory with children - // Return 404 to force clients (like s3fs) to use LIST-based discovery + // This appears to be a directory marker - return 404 to force LIST-based discovery s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) return }