Skip to content

Commit

Permalink
fix: requests should fail for directory objects if key does not end in /
Browse files Browse the repository at this point in the history
The object semantics are that a key that ends with / is different
than a key that does not.  However on posix a name that ends with /
is assumed to be a directory, but it still succeeds to access the
directory by name without a trailing /. So we need to explicitly
check if the request is for a non-directory and we are trying to
access a directory.

Fixes #824
benmcclelland committed Sep 20, 2024
1 parent fba121e commit 20f334b
Showing 3 changed files with 118 additions and 0 deletions.
15 changes: 15 additions & 0 deletions backend/posix/posix.go
Original file line number Diff line number Diff line change
@@ -2326,6 +2326,12 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
// AWS returns success if the object does not exist
return &s3.DeleteObjectOutput{}, nil
}
if !strings.HasSuffix(object, "/") && fi.IsDir() {
// requested object is expecting a file, but the object is a
// directory. treat this as a non-existent object.
// AWS returns success if the object does not exist
return &s3.DeleteObjectOutput{}, nil
}

err = os.Remove(objpath)
if errors.Is(err, fs.ErrNotExist) {
@@ -2491,6 +2497,9 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if !strings.HasSuffix(object, "/") && fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}

if *input.VersionId != "" {
isDelMarker, err := p.isObjDeleteMarker(bucket, object)
@@ -2722,6 +2731,9 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if !strings.HasSuffix(object, "/") && fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}

if *input.VersionId != "" {
isDelMarker, err := p.isObjDeleteMarker(bucket, object)
@@ -2892,6 +2904,9 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
if strings.HasSuffix(srcObject, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if !strings.HasSuffix(srcObject, "/") && fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}

mdmap := make(map[string]string)
p.loadUserMetaData(srcBucket, srcObject, mdmap)
6 changes: 6 additions & 0 deletions tests/integration/group-tests.go
Original file line number Diff line number Diff line change
@@ -142,6 +142,7 @@ func TestHeadObject(s *S3Conf) {
HeadObject_invalid_part_number(s)
HeadObject_non_existing_mp(s)
HeadObject_mp_success(s)
HeadObject_directory_object_noslash(s)
HeadObject_non_existing_dir_object(s)
HeadObject_with_contenttype(s)
HeadObject_success(s)
@@ -155,6 +156,7 @@ func TestGetObjectAttributes(s *S3Conf) {

func TestGetObject(s *S3Conf) {
GetObject_non_existing_key(s)
GetObject_directory_object_noslash(s)
GetObject_invalid_ranges(s)
GetObject_with_meta(s)
GetObject_success(s)
@@ -191,6 +193,7 @@ func TestListObjectsV2(s *S3Conf) {

func TestDeleteObject(s *S3Conf) {
DeleteObject_non_existing_object(s)
DeleteObject_directory_object_noslash(s)
DeleteObject_non_existing_dir_object(s)
DeleteObject_success(s)
DeleteObject_success_status_code(s)
@@ -640,6 +643,7 @@ func GetIntTests() IntTests {
"HeadObject_invalid_part_number": HeadObject_invalid_part_number,
"HeadObject_non_existing_mp": HeadObject_non_existing_mp,
"HeadObject_mp_success": HeadObject_mp_success,
"HeadObject_directory_object_noslash": HeadObject_directory_object_noslash,
"HeadObject_non_existing_dir_object": HeadObject_non_existing_dir_object,
"HeadObject_name_too_long": HeadObject_name_too_long,
"HeadObject_with_contenttype": HeadObject_with_contenttype,
@@ -648,6 +652,7 @@ func GetIntTests() IntTests {
"GetObjectAttributes_non_existing_object": GetObjectAttributes_non_existing_object,
"GetObjectAttributes_existing_object": GetObjectAttributes_existing_object,
"GetObject_non_existing_key": GetObject_non_existing_key,
"GetObject_directory_object_noslash": GetObject_directory_object_noslash,
"GetObject_invalid_ranges": GetObject_invalid_ranges,
"GetObject_with_meta": GetObject_with_meta,
"GetObject_success": GetObject_success,
@@ -675,6 +680,7 @@ func GetIntTests() IntTests {
"ListObjectsV2_all_objs_max_keys": ListObjectsV2_all_objs_max_keys,
"ListObjectsV2_list_all_objs": ListObjectsV2_list_all_objs,
"DeleteObject_non_existing_object": DeleteObject_non_existing_object,
"DeleteObject_directory_object_noslash": DeleteObject_directory_object_noslash,
"DeleteObject_name_too_long": DeleteObject_name_too_long,
"DeleteObject_non_existing_dir_object": DeleteObject_non_existing_dir_object,
"DeleteObject_success": DeleteObject_success,
97 changes: 97 additions & 0 deletions tests/integration/tests.go
Original file line number Diff line number Diff line change
@@ -3009,6 +3009,35 @@ func HeadObject_non_existing_dir_object(s *S3Conf) error {
})
}

func HeadObject_directory_object_noslash(s *S3Conf) error {
testName := "HeadObject_directory_object_noslash"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj/"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}

obj = "my-obj"
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err := checkSdkApiErr(err, "NotFound"); err != nil {
return err
}

return nil
})
}

const defaultContentType = "binary/octet-stream"

func HeadObject_with_contenttype(s *S3Conf) error {
@@ -3229,6 +3258,35 @@ func GetObject_non_existing_key(s *S3Conf) error {
})
}

func GetObject_directory_object_noslash(s *S3Conf) error {
testName := "GetObject_directory_object_noslash"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj/"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}

obj = "my-obj"
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.GetObject(ctx, &s3.GetObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
var bae *types.NoSuchKey
if !errors.As(err, &bae) {
return err
}
return nil
})
}

func GetObject_invalid_ranges(s *S3Conf) error {
testName := "GetObject_invalid_ranges"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -4226,6 +4284,45 @@ func DeleteObject_non_existing_object(s *S3Conf) error {
})
}

func DeleteObject_directory_object_noslash(s *S3Conf) error {
testName := "DeleteObject_directory_object_noslash"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj/"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}

obj = "my-obj"
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}

// the delete above should succeed, but the object should not be deleted
// since it should not correctly match the directory name
// so the below head object should also succeed
obj = "my-obj/"
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
return err
})
}

func DeleteObject_non_existing_dir_object(s *S3Conf) error {
testName := "DeleteObject_non_existing_dir_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {

0 comments on commit 20f334b

Please sign in to comment.