fix: head/get/delete/copy directory object should fail when corresponding file object exists

The API hanlders and backend were stripping trailing "/" in object
paths. So if an object exists and a request came in for head/get/delete/copy
for that same name but with a trailing "/" indicating request should
be for directory object, the "/" would be stripped and the request
would be handlied for the incorrect file object.

This fix adds in checks to handle the case with the training "/"
in the request.

Fixes #709
This commit is contained in:
Ben McClelland
2024-08-05 11:03:41 -07:00
parent cb992a4794
commit 797376a235
5 changed files with 182 additions and 4 deletions

View File

@@ -1523,7 +1523,20 @@ func (p *Posix) DeleteObject(_ context.Context, input *s3.DeleteObjectInput) err
return fmt.Errorf("stat bucket: %w", err)
}
err = os.Remove(filepath.Join(bucket, object))
objpath := filepath.Join(bucket, object)
fi, err := os.Stat(objpath)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil {
return fmt.Errorf("stat object: %w", err)
}
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
err = os.Remove(objpath)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
@@ -1629,6 +1642,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
object := *input.Key
objPath := filepath.Join(bucket, object)
fi, err := os.Stat(objPath)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -1637,6 +1651,10 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
return nil, fmt.Errorf("stat object: %w", err)
}
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
acceptRange := *input.Range
startOffset, length, err := backend.ParseRange(fi.Size(), acceptRange)
if err != nil {
@@ -1801,6 +1819,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
}
objPath := filepath.Join(bucket, object)
fi, err := os.Stat(objPath)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -1808,6 +1827,9 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
if err != nil {
return nil, fmt.Errorf("stat object: %w", err)
}
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
userMetaData := make(map[string]string)
contentType, contentEncoding := p.loadUserMetaData(bucket, object, userMetaData)
@@ -1977,10 +1999,13 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
}
defer f.Close()
fInfo, err := f.Stat()
fi, err := f.Stat()
if err != nil {
return nil, fmt.Errorf("stat object: %w", err)
}
if strings.HasSuffix(srcObject, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
meta := make(map[string]string)
p.loadUserMetaData(srcBucket, srcObject, meta)
@@ -2006,7 +2031,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
}
}
contentLength := fInfo.Size()
contentLength := fi.Size()
etag, err := p.PutObject(ctx,
&s3.PutObjectInput{
@@ -2020,7 +2045,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
return nil, err
}
fi, err := os.Stat(dstObjdPath)
fi, err = os.Stat(dstObjdPath)
if err != nil {
return nil, fmt.Errorf("stat dst object: %w", err)
}

View File

@@ -487,6 +487,7 @@ func (s *ScoutFS) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s
}
objPath := filepath.Join(bucket, object)
fi, err := os.Stat(objPath)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -494,6 +495,9 @@ func (s *ScoutFS) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s
if err != nil {
return nil, fmt.Errorf("stat object: %w", err)
}
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
userMetaData := make(map[string]string)
contentType, contentEncoding := s.loadUserMetaData(bucket, object, userMetaData)
@@ -604,6 +608,7 @@ func (s *ScoutFS) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.Ge
}
objPath := filepath.Join(bucket, object)
fi, err := os.Stat(objPath)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -612,6 +617,10 @@ func (s *ScoutFS) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.Ge
return nil, fmt.Errorf("stat object: %w", err)
}
if strings.HasSuffix(object, "/") && !fi.IsDir() {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
startOffset, length, err := backend.ParseRange(fi.Size(), acceptRange)
if err != nil {
return nil, err

View File

@@ -92,6 +92,10 @@ func (c S3ApiController) GetActions(ctx *fiber.Ctx) error {
if keyEnd != "" {
key = strings.Join([]string{key, keyEnd}, "/")
}
path := ctx.Path()
if path[len(path)-1:] == "/" && key[len(key)-1:] != "/" {
key = key + "/"
}
if ctx.Request().URI().QueryArgs().Has("tagging") {
err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{
@@ -2379,6 +2383,10 @@ func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error {
if keyEnd != "" {
key = strings.Join([]string{key, keyEnd}, "/")
}
path := ctx.Path()
if path[len(path)-1:] == "/" && key[len(key)-1:] != "/" {
key = key + "/"
}
if ctx.Request().URI().QueryArgs().Has("tagging") {
err := auth.VerifyAccess(ctx.Context(), c.be,
@@ -2577,6 +2585,10 @@ func (c S3ApiController) HeadObject(ctx *fiber.Ctx) error {
if keyEnd != "" {
key = strings.Join([]string{key, keyEnd}, "/")
}
path := ctx.Path()
if path[len(path)-1:] == "/" && key[len(key)-1:] != "/" {
key = key + "/"
}
var partNumber *int32
if ctx.Request().URI().QueryArgs().Has("partNumber") {

View File

@@ -142,6 +142,7 @@ func TestHeadObject(s *S3Conf) {
HeadObject_invalid_part_number(s)
HeadObject_non_existing_mp(s)
HeadObject_mp_success(s)
HeadObject_non_existing_dir_object(s)
HeadObject_success(s)
}
@@ -160,6 +161,7 @@ func TestGetObject(s *S3Conf) {
GetObject_success(s)
GetObject_by_range_success(s)
GetObject_by_range_resp_status(s)
GetObject_non_existing_dir_object(s)
}
func TestListObjects(s *S3Conf) {
@@ -182,6 +184,7 @@ func TestListObjectsV2(s *S3Conf) {
func TestDeleteObject(s *S3Conf) {
DeleteObject_non_existing_object(s)
DeleteObject_non_existing_dir_object(s)
DeleteObject_success(s)
DeleteObject_success_status_code(s)
}
@@ -198,6 +201,7 @@ func TestCopyObject(s *S3Conf) {
CopyObject_copy_to_itself(s)
CopyObject_to_itself_with_new_metadata(s)
CopyObject_CopySource_starting_with_slash(s)
CopyObject_non_existing_dir_object(s)
CopyObject_success(s)
}
@@ -576,6 +580,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_non_existing_dir_object": HeadObject_non_existing_dir_object,
"HeadObject_success": HeadObject_success,
"GetObjectAttributes_non_existing_bucket": GetObjectAttributes_non_existing_bucket,
"GetObjectAttributes_non_existing_object": GetObjectAttributes_non_existing_object,
@@ -588,6 +593,7 @@ func GetIntTests() IntTests {
"GetObject_success": GetObject_success,
"GetObject_by_range_success": GetObject_by_range_success,
"GetObject_by_range_resp_status": GetObject_by_range_resp_status,
"GetObject_non_existing_dir_object": GetObject_non_existing_dir_object,
"ListObjects_non_existing_bucket": ListObjects_non_existing_bucket,
"ListObjects_with_prefix": ListObjects_with_prefix,
"ListObject_truncated": ListObject_truncated,
@@ -601,6 +607,7 @@ func GetIntTests() IntTests {
"ListObjectsV2_start_after_not_in_list": ListObjectsV2_start_after_not_in_list,
"ListObjectsV2_start_after_empty_result": ListObjectsV2_start_after_empty_result,
"DeleteObject_non_existing_object": DeleteObject_non_existing_object,
"DeleteObject_non_existing_dir_object": DeleteObject_non_existing_dir_object,
"DeleteObject_success": DeleteObject_success,
"DeleteObject_success_status_code": DeleteObject_success_status_code,
"DeleteObjects_empty_input": DeleteObjects_empty_input,
@@ -611,6 +618,7 @@ func GetIntTests() IntTests {
"CopyObject_copy_to_itself": CopyObject_copy_to_itself,
"CopyObject_to_itself_with_new_metadata": CopyObject_to_itself_with_new_metadata,
"CopyObject_CopySource_starting_with_slash": CopyObject_CopySource_starting_with_slash,
"CopyObject_non_existing_dir_object": CopyObject_non_existing_dir_object,
"CopyObject_success": CopyObject_success,
"PutObjectTagging_non_existing_object": PutObjectTagging_non_existing_object,
"PutObjectTagging_long_tags": PutObjectTagging_long_tags,

View File

@@ -2954,6 +2954,39 @@ func HeadObject_mp_success(s *S3Conf) error {
})
}
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 {
obj, dataLen := "my-obj", int64(1234567)
meta := map[string]string{
"key1": "val1",
"key2": "val2",
}
_, _, err := putObjectWithData(dataLen, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
Metadata: meta,
}, s3client)
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,
})
defer cancel()
if err := checkSdkApiErr(err, "NotFound"); err != nil {
return err
}
return nil
})
}
const defaultContentType = "binary/octet-stream"
func HeadObject_success(s *S3Conf) error {
@@ -3514,6 +3547,33 @@ func GetObject_by_range_resp_status(s *S3Conf) error {
})
}
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 {
dataLength, obj := int64(1234567), "my-obj"
_, _, err := putObjectWithData(dataLength, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
}, s3client)
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,
})
defer cancel()
if err := checkSdkApiErr(err, "NoSuchKey"); err != nil {
return err
}
return nil
})
}
func ListObjects_non_existing_bucket(s *S3Conf) error {
testName := "ListObjects_non_existing_bucket"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -3901,6 +3961,30 @@ func DeleteObject_non_existing_object(s *S3Conf) error {
})
}
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 {
obj := "my-obj"
err := putObjects(s3client, []string{obj}, bucket)
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 := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil {
return err
}
return nil
})
}
func DeleteObject_success(s *S3Conf) error {
testName := "DeleteObject_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -4283,6 +4367,46 @@ func CopyObject_CopySource_starting_with_slash(s *S3Conf) error {
})
}
func CopyObject_non_existing_dir_object(s *S3Conf) error {
testName := "CopyObject_non_existing_dir_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
dataLength, obj := int64(1234567), "my-obj"
dstBucket := getBucketName()
err := setup(s, dstBucket)
if err != nil {
return err
}
_, _, err = putObjectWithData(dataLength, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
}, s3client)
if err != nil {
return err
}
obj = "my-obj/"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: &dstBucket,
Key: &obj,
CopySource: getPtr(fmt.Sprintf("%v/%v", bucket, obj)),
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil {
return err
}
err = teardown(s, dstBucket)
if err != nil {
return nil
}
return nil
})
}
func CopyObject_success(s *S3Conf) error {
testName := "CopyObject_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {