diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 29797be9..a2b3e213 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -598,14 +598,44 @@ func (p *Posix) validateVersionId(versionId string) error { return nil } -func (p *Posix) doesBucketAndObjectExist(bucket, object string) error { - _, err := os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { +// DoesBucketExist returns NoSuchBucket unless bucket names a bucket. Backends +// built on Posix that add their own entry points must check buckets with it +// rather than stat the bucket path themselves. +func (p *Posix) DoesBucketExist(bucket string) error { + return p.doesBucketExist(bucket) +} + +// doesBucketExist returns NoSuchBucket unless bucket names a directory under +// the root, or with bucketlinks a symlink to one: the entries +// listBucketFileInfos returns. The root of a dataset the gateway did not +// create may also hold regular files, symlinks and other entries; a request +// naming one of them fails like one naming a missing bucket instead of +// storing bucket metadata on it or building object paths through it. +func (p *Posix) doesBucketExist(bucket string) error { + path := p.BucketPath(bucket) + fi, err := os.Lstat(path) + if err == nil && p.bucketlinks && fi.Mode()&fs.ModeSymlink != 0 { + fi, err = os.Stat(path) + } + // a symlink loop never resolves to a directory + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ELOOP) { return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) } if err != nil { return fmt.Errorf("stat bucket: %w", err) } + if !fi.IsDir() { + return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) + } + + return nil +} + +func (p *Posix) doesBucketAndObjectExist(bucket, object string) error { + err := p.doesBucketExist(bucket) + if err != nil { + return err + } _, err = os.Stat(p.ObjectPath(bucket, object)) if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { @@ -729,12 +759,9 @@ func (p *Posix) HeadBucket(ctx context.Context, input *s3.HeadBucketInput) (*s3. if !p.isBucketValid(*input.Bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, *input.Bucket) } - _, err = os.Lstat(p.BucketPath(*input.Bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, *input.Bucket) - } + err = p.doesBucketExist(*input.Bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } return &s3.HeadBucketOutput{}, nil @@ -767,6 +794,17 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a err = os.Mkdir(p.BucketPath(bucket), p.newDirPerm) if err != nil && os.IsExist(err) { + // An existing entry that is not a bucket, such as a regular file at + // the root of a preexisting dataset, still takes the name but has + // no bucket metadata to consult. + err := p.doesBucketExist(bucket) + if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)) { + return s3err.GetBucketErr(s3err.ErrBucketAlreadyExists, bucket) + } + if err != nil { + return err + } + aclJSON, err := p.meta.RetrieveAttribute(nil, bucket, "", aclkey) if errors.Is(err, meta.ErrNoSuchKey) { // The directory already exists but has no gateway-managed acl @@ -956,6 +994,10 @@ func (p *Posix) DeleteBucket(ctx context.Context, bucket string) error { if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } + err = p.doesBucketExist(bucket) + if err != nil { + return err + } // Check if the bucket is empty err = p.isBucketEmpty(bucket) if err != nil { @@ -995,12 +1037,9 @@ func (p *Posix) PutBucketOwnershipControls(ctx context.Context, bucket string, o if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } err = p.meta.StoreAttribute(nil, bucket, "", ownershipkey, []byte(ownership)) @@ -1021,12 +1060,9 @@ func (p *Posix) GetBucketOwnershipControls(ctx context.Context, bucket string) ( if !p.isBucketValid(bucket) { return ownship, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return ownship, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return ownship, fmt.Errorf("stat bucket: %w", err) + return ownship, err } ownership, err := p.meta.RetrieveAttribute(nil, bucket, "", ownershipkey) @@ -1049,12 +1085,9 @@ func (p *Posix) DeleteBucketOwnershipControls(ctx context.Context, bucket string if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } err = p.meta.DeleteAttribute(bucket, "", ownershipkey) @@ -1082,12 +1115,9 @@ func (p *Posix) PutBucketVersioning(ctx context.Context, bucket string, status t if !p.versioningEnabled() { return s3err.GetAPIError(s3err.ErrVersioningNotConfigured) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } // Store 1 bit for bucket versioning state @@ -1134,12 +1164,9 @@ func (p *Posix) GetBucketVersioning(ctx context.Context, bucket string) (s3respo return s3response.GetBucketVersioningOutput{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.GetBucketVersioningOutput{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return s3response.GetBucketVersioningOutput{}, fmt.Errorf("stat bucket: %w", err) + return s3response.GetBucketVersioningOutput{}, err } if !p.versioningEnabled() { @@ -1354,12 +1381,9 @@ func (p *Posix) ListObjectVersions(ctx context.Context, input *s3.ListObjectVers max = int(*input.MaxKeys) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.ListVersionsResult{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return s3response.ListVersionsResult{}, fmt.Errorf("stat bucket: %w", err) + return s3response.ListVersionsResult{}, err } fileSystem := os.DirFS(p.BucketPath(bucket)) @@ -1830,12 +1854,9 @@ func (p *Posix) CreateMultipartUpload(ctx context.Context, mpu s3response.Create return s3response.InitiateMultipartUploadResult{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.InitiateMultipartUploadResult{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return s3response.InitiateMultipartUploadResult{}, fmt.Errorf("stat bucket: %w", err) + return s3response.InitiateMultipartUploadResult{}, err } if strings.HasSuffix(*mpu.Key, "/") { @@ -2095,12 +2116,9 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C return res, "", s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err := os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return res, "", s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err := p.doesBucketExist(bucket) if err != nil { - return res, "", fmt.Errorf("stat bucket: %w", err) + return res, "", err } // Rename the upload directory to to atomically claim @@ -3096,12 +3114,9 @@ func (p *Posix) AbortMultipartUpload(ctx context.Context, mpu *s3.AbortMultipart return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } sum := sha256.Sum256([]byte(object)) @@ -3165,12 +3180,9 @@ func (p *Posix) ListMultipartUploads(ctx context.Context, mpu *s3.ListMultipartU } maxUploads := int(*mpu.MaxUploads) - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return lmu, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return lmu, fmt.Errorf("stat bucket: %w", err) + return lmu, err } // ignore readdir error and use the empty list returned @@ -3307,12 +3319,9 @@ func (p *Posix) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3resp } } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return lpr, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return lpr, fmt.Errorf("stat bucket: %w", err) + return lpr, err } sum, err := p.checkUploadIDExists(bucket, object, uploadID) @@ -3463,12 +3472,9 @@ func (p *Posix) UploadPartWithPostFunc(ctx context.Context, input *s3.UploadPart } r := input.Body - _, err := os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err := p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } sum := sha256.Sum256([]byte(object)) @@ -3787,12 +3793,9 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) return s3response.CopyPartResult{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, *upi.Bucket) } - _, err = os.Stat(p.BucketPath(*upi.Bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.CopyPartResult{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, *upi.Bucket) - } + err = p.doesBucketExist(*upi.Bucket) if err != nil { - return s3response.CopyPartResult{}, fmt.Errorf("stat bucket: %w", err) + return s3response.CopyPartResult{}, err } sum := sha256.Sum256([]byte(*upi.Key)) @@ -3822,12 +3825,9 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) return s3response.CopyPartResult{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, srcBucket) } - _, err = os.Stat(p.BucketPath(srcBucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.CopyPartResult{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, srcBucket) - } + err = p.doesBucketExist(srcBucket) if err != nil { - return s3response.CopyPartResult{}, fmt.Errorf("stat bucket: %w", err) + return s3response.CopyPartResult{}, err } if upi.ExpectedSourceBucketOwner != nil && *upi.ExpectedSourceBucketOwner != "" { @@ -4174,12 +4174,9 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje if !p.isBucketValid(*po.Bucket) { return s3response.PutObjectOutput{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, *po.Bucket) } - _, err := os.Stat(p.BucketPath(*po.Bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.PutObjectOutput{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, *po.Bucket) - } + err := p.doesBucketExist(*po.Bucket) if err != nil { - return s3response.PutObjectOutput{}, fmt.Errorf("stat bucket: %w", err) + return s3response.PutObjectOutput{}, err } tags, err := backend.ParseObjectTags(getString(po.Tagging)) @@ -4674,12 +4671,9 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( return nil, err } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } objpath := p.ObjectPath(bucket, object) @@ -5058,7 +5052,16 @@ func (p *Posix) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput) } defer release() - // delete object already checks bucket + bucket := *input.Bucket + if !p.isBucketValid(bucket) { + return s3response.DeleteResult{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) + } + // a missing bucket fails the whole request rather than each key + err = p.doesBucketExist(bucket) + if err != nil { + return s3response.DeleteResult{}, err + } + delResult, errs := []types.DeletedObject{}, []types.Error{} for _, obj := range input.Delete.Objects { //TODO: Make the delete operation concurrent @@ -5119,12 +5122,9 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge if !p.isBucketValid(bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } object := *input.Key @@ -5463,12 +5463,9 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } if versionId != "" { @@ -5784,12 +5781,9 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput return s3response.CopyObjectOutput{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, dstBucket) } - _, err = os.Stat(p.BucketPath(srcBucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.CopyObjectOutput{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, srcBucket) - } + err = p.doesBucketExist(srcBucket) if err != nil { - return s3response.CopyObjectOutput{}, fmt.Errorf("stat bucket: %w", err) + return s3response.CopyObjectOutput{}, err } if input.ExpectedSourceBucketOwner != nil && *input.ExpectedSourceBucketOwner != "" { @@ -5831,12 +5825,9 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput } } - _, err = os.Stat(p.BucketPath(dstBucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.CopyObjectOutput{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, dstBucket) - } + err = p.doesBucketExist(dstBucket) if err != nil { - return s3response.CopyObjectOutput{}, fmt.Errorf("stat bucket: %w", err) + return s3response.CopyObjectOutput{}, err } objPath := joinPathWithTrailer(p.BucketPath(srcBucket), srcObject) @@ -6172,12 +6163,9 @@ func (p *Posix) ListObjectsParametrized(ctx context.Context, input *s3.ListObjec return s3response.ListObjectsResult{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err := os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.ListObjectsResult{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err := p.doesBucketExist(bucket) if err != nil { - return s3response.ListObjectsResult{}, fmt.Errorf("stat bucket: %w", err) + return s3response.ListObjectsResult{}, err } fileSystem := os.DirFS(p.BucketPath(bucket)) @@ -6352,12 +6340,9 @@ func (p *Posix) ListObjectsV2Parametrized(ctx context.Context, input *s3.ListObj return s3response.ListObjectsV2Result{}, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err := os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3response.ListObjectsV2Result{}, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err := p.doesBucketExist(bucket) if err != nil { - return s3response.ListObjectsV2Result{}, fmt.Errorf("stat bucket: %w", err) + return s3response.ListObjectsV2Result{}, err } fileSystem := os.DirFS(p.BucketPath(bucket)) @@ -6394,12 +6379,9 @@ func (p *Posix) PutBucketAcl(ctx context.Context, bucket string, data []byte) er if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } err = p.meta.StoreAttribute(nil, bucket, "", aclkey, data) @@ -6420,12 +6402,9 @@ func (p *Posix) GetBucketAcl(ctx context.Context, input *s3.GetBucketAclInput) ( if !p.isBucketValid(*input.Bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, *input.Bucket) } - _, err = os.Stat(p.BucketPath(*input.Bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, *input.Bucket) - } + err = p.doesBucketExist(*input.Bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } b, err := p.meta.RetrieveAttribute(nil, *input.Bucket, "", aclkey) @@ -6448,12 +6427,9 @@ func (p *Posix) PutBucketTagging(ctx context.Context, bucket string, tags map[st if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } if tags == nil { @@ -6488,12 +6464,9 @@ func (p *Posix) GetBucketTagging(ctx context.Context, bucket string) (map[string if !p.isBucketValid(bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } tags, err := p.getAttrTags(bucket, "", "") @@ -6521,12 +6494,9 @@ func (p *Posix) GetObjectTagging(ctx context.Context, bucket, object, versionId if !p.isBucketValid(bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } if err := p.validateVersionId(versionId); err != nil { @@ -6611,12 +6581,9 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } if err := p.validateVersionId(versionId); err != nil { @@ -6713,12 +6680,9 @@ func (p *Posix) PutBucketPolicy(ctx context.Context, bucket string, policy []byt if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } if policy == nil { @@ -6752,12 +6716,9 @@ func (p *Posix) GetBucketPolicy(ctx context.Context, bucket string) ([]byte, err if !p.isBucketValid(bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } policy, err := p.meta.RetrieveAttribute(nil, bucket, "", policykey) @@ -6791,12 +6752,9 @@ func (p *Posix) PutBucketCors(ctx context.Context, bucket string, cors []byte) e if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } if cors == nil { @@ -6826,12 +6784,9 @@ func (p *Posix) GetBucketCors(ctx context.Context, bucket string) ([]byte, error if !p.isBucketValid(bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } cors, err := p.meta.RetrieveAttribute(nil, bucket, "", corskey) @@ -6862,12 +6817,9 @@ func (p *Posix) PutBucketWebsite(ctx context.Context, bucket string, website []b if !p.isBucketValid(bucket) { return s3err.GetAPIError(s3err.ErrInvalidBucketName) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetAPIError(s3err.ErrNoSuchBucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } if website == nil { @@ -6904,12 +6856,9 @@ func (p *Posix) GetBucketWebsite(ctx context.Context, bucket string) ([]byte, er if !p.isBucketValid(bucket) { return nil, s3err.GetAPIError(s3err.ErrInvalidBucketName) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } website, err := p.meta.RetrieveAttribute(nil, bucket, "", websitekey) @@ -6969,12 +6918,9 @@ func (p *Posix) PutObjectLockConfiguration(ctx context.Context, bucket string, c if !p.isBucketValid(bucket) { return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } if p.versioningEnabled() { @@ -7012,12 +6958,9 @@ func (p *Posix) GetObjectLockConfiguration(ctx context.Context, bucket string) ( if !p.isBucketValid(bucket) { return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err = os.Stat(p.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, bucket) - } + err = p.doesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey) diff --git a/backend/posix/rootentries_test.go b/backend/posix/rootentries_test.go new file mode 100644 index 00000000..e118a9f0 --- /dev/null +++ b/backend/posix/rootentries_test.go @@ -0,0 +1,358 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package posix + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/versity/versitygw/s3err" + "github.com/versity/versitygw/s3response" +) + +// TestRootEntriesThatAreNotBuckets checks that only directories under the +// root, and with BucketLinks symlinks to directories, are buckets. The root of +// a dataset the gateway did not create may hold files and symlinks next to the +// bucket directories: every bucket request naming one fails with NoSuchBucket +// and CreateBucket with BucketAlreadyExists, without changing the entry, +// storing metadata for it or writing through it. +func TestRootEntriesThatAreNotBuckets(t *testing.T) { + for mode, mkMeta := range metaModes(t) { + for _, bucketlinks := range []bool{false, true} { + t.Run(fmt.Sprintf("%v/bucketlinks=%v", mode, bucketlinks), func(t *testing.T) { + root := t.TempDir() + t.Chdir(root) + linked := t.TempDir() + + const fileData = "not a bucket" + if err := os.WriteFile(filepath.Join(root, "file"), []byte(fileData), 0644); err != nil { + t.Fatalf("write root file: %v", err) + } + if err := os.WriteFile(filepath.Join(linked, "obj"), []byte(fileData), 0644); err != nil { + t.Fatalf("write linked directory file: %v", err) + } + for link, target := range map[string]string{ + "linkfile": "file", + "danglink": "missing", + "looplink": "looplink", + "linkdir": linked, + } { + if err := os.Symlink(target, filepath.Join(root, link)); err != nil { + t.Skip(err) + } + } + notBuckets := []string{"file", "linkfile", "danglink", "looplink"} + if !bucketlinks { + notBuckets = append(notBuckets, "linkdir") + } + + storer, opts := mkMeta(t) + opts.BucketLinks = bucketlinks + opts.VersioningDir = t.TempDir() + p, err := New(root, storer, opts) + if err != nil { + t.Fatalf("new posix: %v", err) + } + defer p.Shutdown() + + ctx := context.Background() + bucket, object := "bucket", "obj" + createTestBucket(t, p, bucket) + if _, err := testPut(p, bucket, object, []byte(fileData), nil, nil); err != nil { + t.Fatalf("put object: %v", err) + } + mp, err := p.CreateMultipartUpload(ctx, s3response.CreateMultipartUploadInput{Bucket: &bucket, Key: &object}) + if err != nil { + t.Fatalf("create multipart upload: %v", err) + } + + calls := bucketCalls(ctx, p, bucket, object, mp.UploadId) + for _, name := range notBuckets { + for _, call := range calls { + err := call.run(name) + if !errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)) { + t.Errorf("%v on %q: got %v, want NoSuchBucket", call.name, name, err) + } + } + + err := p.CreateBucket(ctx, &s3.CreateBucketInput{ + Bucket: &name, + CreateBucketConfiguration: &types.CreateBucketConfiguration{}, + }, []byte{}) + if !errors.Is(err, s3err.GetAPIError(s3err.ErrBucketAlreadyExists)) { + t.Errorf("CreateBucket on %q: got %v, want BucketAlreadyExists", name, err) + } + + if _, err := os.Lstat(filepath.Join(root, name)); err != nil { + t.Errorf("root entry %q: %v", name, err) + } + attrs, err := p.meta.ListAttributes(name, "") + if err == nil && len(attrs) != 0 { + t.Errorf("attributes stored for %q: %v", name, attrs) + } + } + + data, err := os.ReadFile(filepath.Join(root, "file")) + if err != nil || string(data) != fileData { + t.Errorf("root file content = %q, %v; want %q", data, err, fileData) + } + if opts.SideCarDir != "" { + ents, err := os.ReadDir(opts.SideCarDir) + if err != nil { + t.Fatalf("read sidecar directory: %v", err) + } + for _, ent := range ents { + if ent.Name() != bucket { + t.Errorf("sidecar metadata stored for %q", ent.Name()) + } + } + } + + if !bucketlinks { + ents, err := os.ReadDir(linked) + if err != nil { + t.Fatalf("read linked directory: %v", err) + } + if len(ents) != 1 || ents[0].Name() != "obj" { + t.Errorf("linked directory entries = %v, want just %q", ents, "obj") + } + return + } + + // With BucketLinks the symlinked directory is a bucket. + linkBucket := "linkdir" + if _, err := p.HeadBucket(ctx, &s3.HeadBucketInput{Bucket: &linkBucket}); err != nil { + t.Fatalf("head symlinked bucket: %v", err) + } + if _, err := testPut(p, linkBucket, "new", []byte(fileData), nil, nil); err != nil { + t.Fatalf("put object in symlinked bucket: %v", err) + } + if _, err := os.Stat(filepath.Join(linked, "new")); err != nil { + t.Fatalf("object not written to the linked directory: %v", err) + } + }) + } + } +} + +type bucketCall struct { + name string + run func(bucket string) error +} + +// bucketCalls returns a call to every backend method that addresses a bucket, +// except CreateBucket. A call runs against the bucket it is passed; the copies +// also run with that bucket as the source, copying object into bucket or into +// the upload uploadID. +func bucketCalls(ctx context.Context, p *Posix, bucket, object, uploadID string) []bucketCall { + key := aws.String(object) + copySource := aws.String(bucket + "/" + object) + body := func() *strings.Reader { return strings.NewReader("data") } + + return []bucketCall{ + {"HeadBucket", func(b string) error { + _, err := p.HeadBucket(ctx, &s3.HeadBucketInput{Bucket: &b}) + return err + }}, + {"DeleteBucket", func(b string) error { + return p.DeleteBucket(ctx, b) + }}, + {"GetBucketAcl", func(b string) error { + _, err := p.GetBucketAcl(ctx, &s3.GetBucketAclInput{Bucket: &b}) + return err + }}, + {"PutBucketAcl", func(b string) error { + return p.PutBucketAcl(ctx, b, []byte("{}")) + }}, + {"ChangeBucketOwner", func(b string) error { + return p.ChangeBucketOwner(ctx, b, "owner") + }}, + {"PutBucketOwnershipControls", func(b string) error { + return p.PutBucketOwnershipControls(ctx, b, types.ObjectOwnershipBucketOwnerEnforced) + }}, + {"GetBucketOwnershipControls", func(b string) error { + _, err := p.GetBucketOwnershipControls(ctx, b) + return err + }}, + {"DeleteBucketOwnershipControls", func(b string) error { + return p.DeleteBucketOwnershipControls(ctx, b) + }}, + {"PutBucketVersioning", func(b string) error { + return p.PutBucketVersioning(ctx, b, types.BucketVersioningStatusEnabled) + }}, + {"GetBucketVersioning", func(b string) error { + _, err := p.GetBucketVersioning(ctx, b) + return err + }}, + {"PutBucketTagging", func(b string) error { + return p.PutBucketTagging(ctx, b, map[string]string{"k": "v"}) + }}, + {"GetBucketTagging", func(b string) error { + _, err := p.GetBucketTagging(ctx, b) + return err + }}, + {"DeleteBucketTagging", func(b string) error { + return p.DeleteBucketTagging(ctx, b) + }}, + {"PutBucketPolicy", func(b string) error { + return p.PutBucketPolicy(ctx, b, []byte("{}")) + }}, + {"GetBucketPolicy", func(b string) error { + _, err := p.GetBucketPolicy(ctx, b) + return err + }}, + {"DeleteBucketPolicy", func(b string) error { + return p.DeleteBucketPolicy(ctx, b) + }}, + {"PutBucketCors", func(b string) error { + return p.PutBucketCors(ctx, b, []byte("")) + }}, + {"GetBucketCors", func(b string) error { + _, err := p.GetBucketCors(ctx, b) + return err + }}, + {"DeleteBucketCors", func(b string) error { + return p.DeleteBucketCors(ctx, b) + }}, + {"PutBucketWebsite", func(b string) error { + return p.PutBucketWebsite(ctx, b, []byte("")) + }}, + {"GetBucketWebsite", func(b string) error { + _, err := p.GetBucketWebsite(ctx, b) + return err + }}, + {"DeleteBucketWebsite", func(b string) error { + return p.DeleteBucketWebsite(ctx, b) + }}, + {"PutObjectLockConfiguration", func(b string) error { + return p.PutObjectLockConfiguration(ctx, b, []byte(`{"Enabled":true}`)) + }}, + {"GetObjectLockConfiguration", func(b string) error { + _, err := p.GetObjectLockConfiguration(ctx, b) + return err + }}, + {"ListObjects", func(b string) error { + _, err := p.ListObjects(ctx, &s3.ListObjectsInput{Bucket: &b, MaxKeys: aws.Int32(1000)}) + return err + }}, + {"ListObjectsV2", func(b string) error { + _, err := p.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: &b, MaxKeys: aws.Int32(1000), StartAfter: aws.String("")}) + return err + }}, + {"ListObjectVersions", func(b string) error { + _, err := p.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{Bucket: &b, MaxKeys: aws.Int32(1000)}) + return err + }}, + {"ListMultipartUploads", func(b string) error { + _, err := p.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{Bucket: &b, MaxUploads: aws.Int32(1000)}) + return err + }}, + {"PutObject", func(b string) error { + _, err := p.PutObject(ctx, s3response.PutObjectInput{Bucket: &b, Key: key, Body: body(), ContentLength: aws.Int64(4)}) + return err + }}, + {"PutObjectDirectory", func(b string) error { + _, err := p.PutObject(ctx, s3response.PutObjectInput{Bucket: &b, Key: aws.String("dir/"), Body: strings.NewReader(""), ContentLength: aws.Int64(0)}) + return err + }}, + {"GetObject", func(b string) error { + _, err := p.GetObject(ctx, &s3.GetObjectInput{Bucket: &b, Key: key}) + return err + }}, + {"HeadObject", func(b string) error { + _, err := p.HeadObject(ctx, &s3.HeadObjectInput{Bucket: &b, Key: key}) + return err + }}, + {"GetObjectAttributes", func(b string) error { + _, err := p.GetObjectAttributes(ctx, &s3.GetObjectAttributesInput{Bucket: &b, Key: key}) + return err + }}, + {"DeleteObject", func(b string) error { + _, err := p.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: &b, Key: key}) + return err + }}, + {"DeleteObjects", func(b string) error { + _, err := p.DeleteObjects(ctx, &s3.DeleteObjectsInput{Bucket: &b, Delete: &types.Delete{Objects: []types.ObjectIdentifier{{Key: key}}}}) + return err + }}, + {"CopyObjectDestination", func(b string) error { + _, err := p.CopyObject(ctx, s3response.CopyObjectInput{Bucket: &b, Key: key, CopySource: copySource, ExpectedBucketOwner: aws.String("")}) + return err + }}, + {"CopyObjectSource", func(b string) error { + _, err := p.CopyObject(ctx, s3response.CopyObjectInput{Bucket: &bucket, Key: aws.String("copy"), CopySource: aws.String(b + "/" + object), ExpectedBucketOwner: aws.String("")}) + return err + }}, + {"PutObjectTagging", func(b string) error { + return p.PutObjectTagging(ctx, b, object, "", map[string]string{"k": "v"}) + }}, + {"GetObjectTagging", func(b string) error { + _, err := p.GetObjectTagging(ctx, b, object, "") + return err + }}, + {"DeleteObjectTagging", func(b string) error { + return p.DeleteObjectTagging(ctx, b, object, "") + }}, + {"PutObjectLegalHold", func(b string) error { + return p.PutObjectLegalHold(ctx, b, object, "", true) + }}, + {"GetObjectLegalHold", func(b string) error { + _, err := p.GetObjectLegalHold(ctx, b, object, "") + return err + }}, + {"PutObjectRetention", func(b string) error { + return p.PutObjectRetention(ctx, b, object, "", []byte(`{"Mode":"GOVERNANCE"}`)) + }}, + {"GetObjectRetention", func(b string) error { + _, err := p.GetObjectRetention(ctx, b, object, "") + return err + }}, + {"CreateMultipartUpload", func(b string) error { + _, err := p.CreateMultipartUpload(ctx, s3response.CreateMultipartUploadInput{Bucket: &b, Key: key}) + return err + }}, + {"UploadPart", func(b string) error { + _, err := p.UploadPart(ctx, &s3.UploadPartInput{Bucket: &b, Key: key, UploadId: &uploadID, PartNumber: aws.Int32(1), Body: body(), ContentLength: aws.Int64(4)}) + return err + }}, + {"UploadPartCopyDestination", func(b string) error { + _, err := p.UploadPartCopy(ctx, &s3.UploadPartCopyInput{Bucket: &b, Key: key, UploadId: &uploadID, PartNumber: aws.Int32(1), CopySource: copySource, CopySourceRange: aws.String("")}) + return err + }}, + {"UploadPartCopySource", func(b string) error { + _, err := p.UploadPartCopy(ctx, &s3.UploadPartCopyInput{Bucket: &bucket, Key: key, UploadId: &uploadID, PartNumber: aws.Int32(1), CopySource: aws.String(b + "/" + object), CopySourceRange: aws.String("")}) + return err + }}, + {"ListParts", func(b string) error { + _, err := p.ListParts(ctx, &s3.ListPartsInput{Bucket: &b, Key: key, UploadId: &uploadID, MaxParts: aws.Int32(1000)}) + return err + }}, + {"CompleteMultipartUpload", func(b string) error { + _, _, err := p.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{Bucket: &b, Key: key, UploadId: &uploadID, MultipartUpload: &types.CompletedMultipartUpload{}}) + return err + }}, + {"AbortMultipartUpload", func(b string) error { + return p.AbortMultipartUpload(ctx, &s3.AbortMultipartUploadInput{Bucket: &b, Key: key, UploadId: &uploadID}) + }}, + } +} diff --git a/backend/scoutfs/scoutfs_compat.go b/backend/scoutfs/scoutfs_compat.go index aaefaeac..ab702d94 100644 --- a/backend/scoutfs/scoutfs_compat.go +++ b/backend/scoutfs/scoutfs_compat.go @@ -351,12 +351,9 @@ func (s *ScoutFS) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3. return nil, s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err := os.Stat(s.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return nil, s3err.GetBucketErr(s3err.ErrNoSuchBucket, *input.Bucket) - } + err := s.DoesBucketExist(bucket) if err != nil { - return nil, fmt.Errorf("stat bucket: %w", err) + return nil, err } objPath := s.ObjectPath(bucket, object) @@ -446,12 +443,9 @@ func (s *ScoutFS) RestoreObject(_ context.Context, input *s3.RestoreObjectInput) return s3err.GetBucketErr(s3err.ErrInvalidBucketName, bucket) } - _, err := os.Stat(s.BucketPath(bucket)) - if errors.Is(err, fs.ErrNotExist) { - return s3err.GetBucketErr(s3err.ErrNoSuchBucket, *input.Bucket) - } + err := s.DoesBucketExist(bucket) if err != nil { - return fmt.Errorf("stat bucket: %w", err) + return err } err = setStaging(s.ObjectPath(bucket, object)) diff --git a/tests/integration/DeleteObjects.go b/tests/integration/DeleteObjects.go index d97b1e37..afada241 100644 --- a/tests/integration/DeleteObjects.go +++ b/tests/integration/DeleteObjects.go @@ -24,6 +24,22 @@ import ( "github.com/versity/versitygw/s3err" ) +func DeleteObjects_non_existing_bucket(s *S3Conf) error { + testName := "DeleteObjects_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + bckt := getBucketName() + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{ + Bucket: &bckt, + Delete: &types.Delete{ + Objects: []types.ObjectIdentifier{{Key: getPtr("obj1")}, {Key: getPtr("obj2")}}, + }, + }) + cancel() + return checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)) + }) +} + func DeleteObjects_empty_input(s *S3Conf) error { testName := "DeleteObjects_empty_input" 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 90825d68..78b7e4c5 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -357,6 +357,7 @@ func TestDeleteObject(ts *TestState) { } func TestDeleteObjects(ts *TestState) { + ts.Run(DeleteObjects_non_existing_bucket) ts.Run(DeleteObjects_empty_input) ts.Run(DeleteObjects_non_existing_objects) ts.Run(DeleteObjects_success) @@ -2988,6 +2989,7 @@ func GetIntTests() IntTests { "DeleteObject_empty_version_id": DeleteObject_empty_version_id, "DeleteObject_incorrect_expected_bucket_owner": DeleteObject_incorrect_expected_bucket_owner, "DeleteObject_expected_bucket_owner": DeleteObject_expected_bucket_owner, + "DeleteObjects_non_existing_bucket": DeleteObjects_non_existing_bucket, "DeleteObjects_empty_input": DeleteObjects_empty_input, "DeleteObjects_non_existing_objects": DeleteObjects_non_existing_objects, "DeleteObjects_success": DeleteObjects_success,