fix: internal server error when object parent dir is a file

The fileystem will return ENOTDIR if we try to access a file path
where a parent directory within the path already exists as a file.
In this case we need to return a standard 404 no such key since
the request object does not exist within the filesytem.

Fixes #942
This commit is contained in:
Ben McClelland
2024-11-07 21:22:11 -08:00
parent b4190f6749
commit 0312a1e3dc
4 changed files with 169 additions and 25 deletions

View File

@@ -198,7 +198,7 @@ func (p *Posix) doesBucketAndObjectExist(bucket, object string) error {
}
_, err = os.Stat(filepath.Join(bucket, object))
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil {
@@ -761,7 +761,7 @@ func getBoolPtr(b bool) *bool {
// Check if the given object is a delete marker
func (p *Posix) isObjDeleteMarker(bucket, object string) (bool, error) {
_, err := p.meta.RetrieveAttribute(nil, bucket, object, deleteMarkerKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return false, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if errors.Is(err, meta.ErrNoSuchKey) {
@@ -2434,7 +2434,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
if getString(input.VersionId) == "" {
// if the versionId is not specified, make the current version a delete marker
fi, err := os.Stat(objpath)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
// AWS returns success if the object does not exist
return &s3.DeleteObjectOutput{}, nil
}
@@ -2601,7 +2601,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
if errors.Is(err, syscall.ENAMETOOLONG) {
return nil, s3err.GetAPIError(s3err.ErrKeyTooLong)
}
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
if err != nil {
@@ -2621,7 +2621,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
if errors.Is(err, syscall.ENAMETOOLONG) {
return nil, s3err.GetAPIError(s3err.ErrKeyTooLong)
}
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
// AWS returns success if the object does not exist
return &s3.DeleteObjectOutput{}, nil
}
@@ -2768,7 +2768,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
object := *input.Key
if versionId != "" {
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -2787,7 +2787,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput) (*s3.GetO
objPath := filepath.Join(bucket, object)
fi, err := os.Stat(objPath)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if versionId != "" {
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
@@ -3025,7 +3025,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3.
if versionId != "" {
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -3045,7 +3045,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) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if versionId != "" {
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
@@ -3206,7 +3206,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
vId, err := p.meta.RetrieveAttribute(nil, srcBucket, srcObject, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -3229,7 +3229,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
objPath := filepath.Join(srcBucket, srcObject)
f, err := os.Open(objPath)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if p.versioningEnabled() && vEnabled {
return nil, s3err.GetAPIError(s3err.ErrNoSuchVersion)
}
@@ -3284,7 +3284,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3.
b, _ := p.meta.RetrieveAttribute(nil, dstBucket, dstObject, etagkey)
etag = string(b)
vId, _ := p.meta.RetrieveAttribute(nil, dstBucket, dstObject, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
version = backend.GetPtrFromString(string(vId))
@@ -3612,7 +3612,7 @@ func (p *Posix) GetObjectTagging(_ context.Context, bucket, object string) (map[
func (p *Posix) getAttrTags(bucket, object string) (map[string]string, error) {
tags := make(map[string]string)
b, err := p.meta.RetrieveAttribute(nil, bucket, object, tagHdr)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if errors.Is(err, meta.ErrNoSuchKey) {
@@ -3641,7 +3641,7 @@ func (p *Posix) PutObjectTagging(_ context.Context, bucket, object string, tags
if tags == nil {
err = p.meta.DeleteAttribute(bucket, object, tagHdr)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if errors.Is(err, meta.ErrNoSuchKey) {
@@ -3659,7 +3659,7 @@ func (p *Posix) PutObjectTagging(_ context.Context, bucket, object string, tags
}
err = p.meta.StoreAttribute(nil, bucket, object, tagHdr, b)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil {
@@ -3831,7 +3831,7 @@ func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -3845,7 +3845,7 @@ func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId
}
err = p.meta.StoreAttribute(nil, bucket, object, objectLegalHoldKey, statusData)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if versionId != "" {
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
@@ -3874,7 +3874,7 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -3888,7 +3888,7 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId
}
data, err := p.meta.RetrieveAttribute(nil, bucket, object, objectLegalHoldKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if versionId != "" {
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
@@ -3922,7 +3922,7 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -3936,7 +3936,7 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId
}
objectLockCfg, err := p.meta.RetrieveAttribute(nil, bucket, object, objectRetentionKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if versionId != "" {
return s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
@@ -3994,7 +3994,7 @@ func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}
vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
@@ -4008,7 +4008,7 @@ func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId
}
data, err := p.meta.RetrieveAttribute(nil, bucket, object, objectRetentionKey)
if errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
if versionId != "" {
return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
}

View File

@@ -490,7 +490,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) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if errors.Is(err, syscall.ENAMETOOLONG) {
@@ -614,7 +614,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) {
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if errors.Is(err, syscall.ENAMETOOLONG) {

View File

@@ -152,6 +152,7 @@ func TestHeadObject(s *S3Conf) {
HeadObject_directory_object_noslash(s)
HeadObject_non_existing_dir_object(s)
HeadObject_with_contenttype(s)
HeadObject_invalid_parent_dir(s)
HeadObject_success(s)
}
@@ -159,6 +160,7 @@ func TestGetObjectAttributes(s *S3Conf) {
GetObjectAttributes_non_existing_bucket(s)
GetObjectAttributes_non_existing_object(s)
GetObjectAttributes_invalid_attrs(s)
GetObjectAttributes_invalid_parent(s)
GetObjectAttributes_empty_attrs(s)
GetObjectAttributes_existing_object(s)
}
@@ -167,6 +169,7 @@ func TestGetObject(s *S3Conf) {
GetObject_non_existing_key(s)
GetObject_directory_object_noslash(s)
GetObject_invalid_ranges(s)
GetObject_invalid_parent(s)
GetObject_with_meta(s)
GetObject_success(s)
GetObject_directory_success(s)
@@ -239,6 +242,7 @@ func TestPutObjectTagging(s *S3Conf) {
func TestGetObjectTagging(s *S3Conf) {
GetObjectTagging_non_existing_object(s)
GetObjectTagging_unset_tags(s)
GetObjectTagging_invalid_parent(s)
GetObjectTagging_success(s)
}
@@ -567,6 +571,7 @@ func TestVersioning(s *S3Conf) {
Versioning_CopyObject_special_chars(s)
// HeadObject action
Versioning_HeadObject_invalid_versionId(s)
Versioning_HeadObject_invalid_parent(s)
Versioning_HeadObject_success(s)
Versioning_HeadObject_without_versionId(s)
Versioning_HeadObject_delete_marker(s)
@@ -725,15 +730,18 @@ func GetIntTests() IntTests {
"HeadObject_non_existing_dir_object": HeadObject_non_existing_dir_object,
"HeadObject_name_too_long": HeadObject_name_too_long,
"HeadObject_with_contenttype": HeadObject_with_contenttype,
"HeadObject_invalid_parent_dir": HeadObject_invalid_parent_dir,
"HeadObject_success": HeadObject_success,
"GetObjectAttributes_non_existing_bucket": GetObjectAttributes_non_existing_bucket,
"GetObjectAttributes_non_existing_object": GetObjectAttributes_non_existing_object,
"GetObjectAttributes_invalid_attrs": GetObjectAttributes_invalid_attrs,
"GetObjectAttributes_invalid_parent": GetObjectAttributes_invalid_parent,
"GetObjectAttributes_empty_attrs": GetObjectAttributes_empty_attrs,
"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_invalid_parent": GetObject_invalid_parent,
"GetObject_with_meta": GetObject_with_meta,
"GetObject_success": GetObject_success,
"GetObject_directory_success": GetObject_directory_success,
@@ -782,6 +790,7 @@ func GetIntTests() IntTests {
"PutObjectTagging_success": PutObjectTagging_success,
"GetObjectTagging_non_existing_object": GetObjectTagging_non_existing_object,
"GetObjectTagging_unset_tags": GetObjectTagging_unset_tags,
"GetObjectTagging_invalid_parent": GetObjectTagging_invalid_parent,
"GetObjectTagging_success": GetObjectTagging_success,
"DeleteObjectTagging_non_existing_object": DeleteObjectTagging_non_existing_object,
"DeleteObjectTagging_success_status": DeleteObjectTagging_success_status,
@@ -969,6 +978,7 @@ func GetIntTests() IntTests {
"Versioning_CopyObject_from_an_object_version": Versioning_CopyObject_from_an_object_version,
"Versioning_CopyObject_special_chars": Versioning_CopyObject_special_chars,
"Versioning_HeadObject_invalid_versionId": Versioning_HeadObject_invalid_versionId,
"Versioning_HeadObject_invalid_parent": Versioning_HeadObject_invalid_parent,
"Versioning_HeadObject_success": Versioning_HeadObject_success,
"Versioning_HeadObject_without_versionId": Versioning_HeadObject_without_versionId,
"Versioning_HeadObject_delete_marker": Versioning_HeadObject_delete_marker,

View File

@@ -3340,6 +3340,34 @@ func HeadObject_with_contenttype(s *S3Conf) error {
})
}
func HeadObject_invalid_parent_dir(s *S3Conf) error {
testName := "HeadObject_invalid_parent_dir"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj, dataLen := "not-a-dir", int64(1)
_, err := putObjectWithData(dataLen, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
}, s3client)
if err != nil {
return err
}
obj = "not-a-dir/bad-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
})
}
func HeadObject_success(s *S3Conf) error {
testName := "HeadObject_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -3456,6 +3484,34 @@ func GetObjectAttributes_invalid_attrs(s *S3Conf) error {
})
}
func GetObjectAttributes_invalid_parent(s *S3Conf) error {
testName := "GetObjectAttributes_invalid_parent"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "not-a-dir"
_, err := putObjects(s3client, []string{obj}, bucket)
if err != nil {
return err
}
obj = "not-a-dir/bad-obj"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{
Bucket: &bucket,
Key: &obj,
ObjectAttributes: []types.ObjectAttributes{
types.ObjectAttributesEtag,
},
})
cancel()
var bae *types.NoSuchKey
if !errors.As(err, &bae) {
return err
}
return nil
})
}
func GetObjectAttributes_empty_attrs(s *S3Conf) error {
testName := "GetObjectAttributes_empty_attrs"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -3642,6 +3698,33 @@ func GetObject_invalid_ranges(s *S3Conf) error {
})
}
func GetObject_invalid_parent(s *S3Conf) error {
testName := "GetObject_invalid_parent"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
dataLength, obj := int64(1234567), "not-a-dir"
_, err := putObjectWithData(dataLength, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
}, s3client)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.GetObject(ctx, &s3.GetObjectInput{
Bucket: &bucket,
Key: getPtr("not-a-dir/bad-obj"),
})
cancel()
var bae *types.NoSuchKey
if !errors.As(err, &bae) {
return err
}
return nil
})
}
func GetObject_with_meta(s *S3Conf) error {
testName := "GetObject_with_meta"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -5349,6 +5432,29 @@ func GetObjectTagging_unset_tags(s *S3Conf) error {
})
}
func GetObjectTagging_invalid_parent(s *S3Conf) error {
testName := "GetObjectTagging_invalid_parent"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "not-a-dir"
_, err := putObjects(s3client, []string{obj}, bucket)
if err != nil {
return err
}
obj = "not-a-dir/bad-obj"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.GetObjectTagging(ctx, &s3.GetObjectTaggingInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil {
return err
}
return nil
})
}
func GetObjectTagging_success(s *S3Conf) error {
testName := "PutObjectTagging_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -11516,6 +11622,34 @@ func Versioning_HeadObject_invalid_versionId(s *S3Conf) error {
})
}
func Versioning_HeadObject_invalid_parent(s *S3Conf) error {
testName := "Versioning_HeadObject_invalid_parent"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
dLen := int64(2000)
obj := "not-a-dir"
r, err := putObjectWithData(dLen, &s3.PutObjectInput{
Bucket: &bucket,
Key: &obj,
}, s3client)
if err != nil {
return err
}
obj = "not-a-dir/bad-obj"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
VersionId: r.res.VersionId,
})
cancel()
if err := checkSdkApiErr(err, "NotFound"); err != nil {
return err
}
return nil
})
}
func Versioning_HeadObject_success(s *S3Conf) error {
testName := "Versioning_HeadObject_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {