From 2c0844ad88705f28943f531e1608aee6f37e7404 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Tue, 12 May 2026 18:50:14 -0700 Subject: [PATCH] fix: ignore implicit directories for Get/HeadObject Directories that exist on the filesystem but were not explicitly created via S3 (put-object with a key ending in '/') do not have an etag value. ListObjectsV2 already uses the presence of this attribute to decide whether to include a directory as an object. GetObject and HeadObject were not performing this check, so they would successfully return directories that ListObjectsV2 would not list. Add the etag attribute check in GetObject and HeadObject: if a directory path is requested but has no etag xattr, return 404. This makes all three operations agree on which directories are S3 objects. Fixes #2130 --- backend/posix/posix.go | 26 ++++++++++++++++++++++++++ tests/integration/GetObject.go | 28 ++++++++++++++++++++++++++++ tests/integration/HeadObject.go | 28 ++++++++++++++++++++++++++++ tests/integration/group-tests.go | 4 ++++ 4 files changed, 86 insertions(+) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index f3584e9a..186e4845 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -4496,6 +4496,19 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge if !strings.HasSuffix(object, "/") && fid.IsDir() { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } + if fid.IsDir() { + // Only directories explicitly created via S3 (put-object with key ending + // in '/') have an etag attribute. Directories created incidentally on the + // filesystem or as parent directories during object upload should not be + // accessible via get-object. + _, derr := p.meta.RetrieveAttribute(nil, bucket, object, etagkey) + if errors.Is(derr, meta.ErrNoSuchKey) || errors.Is(derr, fs.ErrNotExist) { + return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) + } + if derr != nil { + return nil, fmt.Errorf("get dir etag: %w", derr) + } + } if p.versioningEnabled() { isDelMarker, err := p.isObjDeleteMarker(bucket, object) @@ -4821,6 +4834,19 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. if !strings.HasSuffix(object, "/") && fi.IsDir() { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } + if fi.IsDir() { + // Only directories explicitly created via S3 (put-object with key ending + // in '/') have an etag attribute. Directories created incidentally on the + // filesystem or as parent directories during object upload should not be + // accessible via head-object. + _, derr := p.meta.RetrieveAttribute(nil, bucket, object, etagkey) + if errors.Is(derr, meta.ErrNoSuchKey) || errors.Is(derr, fs.ErrNotExist) { + return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) + } + if derr != nil { + return nil, fmt.Errorf("get dir etag: %w", derr) + } + } if p.versioningEnabled() { isDelMarker, err := p.isObjDeleteMarker(bucket, object) diff --git a/tests/integration/GetObject.go b/tests/integration/GetObject.go index 7df37b39..25cac77d 100644 --- a/tests/integration/GetObject.go +++ b/tests/integration/GetObject.go @@ -829,6 +829,34 @@ func GetObject_not_enabled_checksum_mode(s *S3Conf) error { }) } +// GetObject_incidental_dir_object verifies that a directory created incidentally +// as a parent during object upload (i.e. never explicitly PUT via S3 with a +// trailing-slash key) is not accessible via GetObject. +func GetObject_incidental_dir_object(s *S3Conf) error { + testName := "GetObject_incidental_dir_object" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + // Upload an object under a prefix; this creates the parent directory + // incidentally on posix but the directory was never PUT as an S3 object. + obj := "my-dir/my-obj" + _, err := putObjectWithData(int64(64), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + dir := "my-dir/" + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObject(ctx, &s3.GetObjectInput{ + Bucket: &bucket, + Key: &dir, + }) + cancel() + return checkSdkApiErr(err, "NoSuchKey") + }) +} + func GetObject_non_existing_dir_object(s *S3Conf) error { testName := "GetObject_non_existing_dir_object" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/HeadObject.go b/tests/integration/HeadObject.go index b89a8e72..b3f504da 100644 --- a/tests/integration/HeadObject.go +++ b/tests/integration/HeadObject.go @@ -62,6 +62,34 @@ func HeadObject_invalid_part_number(s *S3Conf) error { }) } +// HeadObject_incidental_dir_object verifies that a directory created incidentally +// as a parent during object upload (i.e. never explicitly PUT via S3 with a +// trailing-slash key) is not accessible via HeadObject. +func HeadObject_incidental_dir_object(s *S3Conf) error { + testName := "HeadObject_incidental_dir_object" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + // Upload an object under a prefix; this creates the parent directory + // incidentally on posix but the directory was never PUT as an S3 object. + obj := "my-dir/my-obj" + _, err := putObjectWithData(int64(64), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + dir := "my-dir/" + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &dir, + }) + cancel() + return checkSdkApiErr(err, "NotFound") + }) +} + func HeadObject_non_existing_dir_object(s *S3Conf) error { testName := "HeadObject_non_existing_dir_object" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 49e0e74a..61100b53 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -205,6 +205,7 @@ func TestHeadObject(ts *TestState) { ts.Run(HeadObject_invalid_part_number) ts.Run(HeadObject_directory_object_noslash) ts.Run(HeadObject_non_existing_dir_object) + ts.Run(HeadObject_incidental_dir_object) ts.Run(HeadObject_invalid_parent_dir) ts.Run(HeadObject_with_range) ts.Run(HeadObject_by_range_resp_status) @@ -263,6 +264,7 @@ func TestGetObject(ts *TestState) { ts.Run(GetObject_directory_success) ts.Run(GetObject_by_range_resp_status) ts.Run(GetObject_non_existing_dir_object) + ts.Run(GetObject_incidental_dir_object) ts.Run(GetObject_overrides_success) ts.Run(GetObject_overrides_presign_success) ts.Run(GetObject_overrides_fail_public) @@ -1412,6 +1414,7 @@ func GetIntTests() IntTests { "HeadObject_invalid_part_number": HeadObject_invalid_part_number, "HeadObject_directory_object_noslash": HeadObject_directory_object_noslash, "HeadObject_non_existing_dir_object": HeadObject_non_existing_dir_object, + "HeadObject_incidental_dir_object": HeadObject_incidental_dir_object, "HeadObject_name_too_long": HeadObject_name_too_long, "HeadObject_invalid_parent_dir": HeadObject_invalid_parent_dir, "HeadObject_with_range": HeadObject_with_range, @@ -1456,6 +1459,7 @@ func GetIntTests() IntTests { "GetObject_directory_success": GetObject_directory_success, "GetObject_by_range_resp_status": GetObject_by_range_resp_status, "GetObject_non_existing_dir_object": GetObject_non_existing_dir_object, + "GetObject_incidental_dir_object": GetObject_incidental_dir_object, "GetObject_overrides_success": GetObject_overrides_success, "GetObject_overrides_presign_success": GetObject_overrides_presign_success, "GetObject_overrides_fail_public": GetObject_overrides_fail_public,