From 32c6f2e463c9730c1fd4385c5bacb368fdf2de41 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Tue, 20 May 2025 13:43:26 -0700 Subject: [PATCH] fix: non existing bucket acl parsing There were a couple of cases that would return an error for the non existing bucket acl instead of treating that as the default acl. This also cleans up the backends that were doing their own acl parsing instead of using the auth.ParseACL() function. Fixes #1304 --- backend/azure/azure.go | 31 ++++++++++++------------------- backend/posix/posix.go | 34 ++++++++++++++++------------------ backend/s3proxy/s3.go | 7 ++++--- 3 files changed, 32 insertions(+), 40 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 46a7cc3..5cfa638 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -181,11 +181,9 @@ func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, return err } - var acl auth.ACL - if len(aclBytes) > 0 { - if err := json.Unmarshal(aclBytes, &acl); err != nil { - return fmt.Errorf("unmarshal acl: %w", err) - } + acl, err := auth.ParseACL(aclBytes) + if err != nil { + return err } if acl.Owner == acct.Access { @@ -607,9 +605,9 @@ func (az *Azure) ListObjects(ctx context.Context, input *s3.ListObjectsInput) (s return s3response.ListObjectsResult{}, azureErrToS3Err(err) } - var acl auth.ACL - if err := json.Unmarshal(aclBytes, &acl); err != nil { - return s3response.ListObjectsResult{}, fmt.Errorf("unmarshal acl: %w", err) + acl, err := auth.ParseACL(aclBytes) + if err != nil { + return s3response.ListObjectsResult{}, err } Pager: @@ -710,8 +708,9 @@ func (az *Azure) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input return s3response.ListObjectsV2Result{}, azureErrToS3Err(err) } - if err := json.Unmarshal(aclBytes, &acl); err != nil { - return s3response.ListObjectsV2Result{}, fmt.Errorf("unmarshal acl: %w", err) + acl, err = auth.ParseACL(aclBytes) + if err != nil { + return s3response.ListObjectsV2Result{}, err } } @@ -1965,11 +1964,9 @@ func (az *Azure) deleteContainerMetaData(ctx context.Context, bucket, key string } func getAclFromMetadata(meta map[string]*string, key key) (*auth.ACL, error) { - var acl auth.ACL - data, ok := meta[string(key)] if !ok { - return &acl, nil + return &auth.ACL{}, nil } value, err := decodeString(*data) @@ -1977,13 +1974,9 @@ func getAclFromMetadata(meta map[string]*string, key key) (*auth.ACL, error) { return nil, err } - if len(value) == 0 { - return &acl, nil - } - - err = json.Unmarshal(value, &acl) + acl, err := auth.ParseACL(value) if err != nil { - return nil, fmt.Errorf("unmarshal acl: %w", err) + return nil, err } return &acl, nil diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 7014c15..76b1e87 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -299,7 +299,7 @@ func (p *Posix) ListBuckets(_ context.Context, input s3response.ListBucketsInput continue } - aclTag, err := p.meta.RetrieveAttribute(nil, fi.Name(), "", aclkey) + aclJSON, err := p.meta.RetrieveAttribute(nil, fi.Name(), "", aclkey) if errors.Is(err, meta.ErrNoSuchKey) { // skip buckets without acl tag continue @@ -308,10 +308,9 @@ func (p *Posix) ListBuckets(_ context.Context, input s3response.ListBucketsInput return s3response.ListAllMyBucketsResult{}, fmt.Errorf("get acl tag: %w", err) } - var acl auth.ACL - err = json.Unmarshal(aclTag, &acl) + acl, err := auth.ParseACL(aclJSON) if err != nil { - return s3response.ListAllMyBucketsResult{}, fmt.Errorf("parse acl tag: %w", err) + return s3response.ListAllMyBucketsResult{}, err } if acl.Owner == input.Owner { @@ -370,9 +369,10 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a if err != nil { return fmt.Errorf("get bucket acl: %w", err) } - var acl auth.ACL - if err := json.Unmarshal(aclJSON, &acl); err != nil { - return fmt.Errorf("unmarshal acl: %w", err) + + acl, err := auth.ParseACL(aclJSON) + if err != nil { + return err } if acl.Owner == acct.Access { @@ -4233,12 +4233,13 @@ func (p *Posix) fileToObj(bucket string, fetchOwner bool) backend.GetObjFunc { // All the objects in the bucket are owned by the bucket owner if fetchOwner { aclJSON, err := p.meta.RetrieveAttribute(nil, bucket, "", aclkey) - if err != nil { + if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return s3response.Object{}, fmt.Errorf("get bucket acl: %w", err) } - var acl auth.ACL - if err := json.Unmarshal(aclJSON, &acl); err != nil { - return s3response.Object{}, fmt.Errorf("unmarshal acl: %w", err) + + acl, err := auth.ParseACL(aclJSON) + if err != nil { + return s3response.Object{}, err } owner = &types.Owner{ @@ -4950,17 +4951,14 @@ func (p *Posix) ListBucketsAndOwners(ctx context.Context) (buckets []s3response. } for _, fi := range fis { - aclTag, err := p.meta.RetrieveAttribute(nil, fi.Name(), "", aclkey) + aclJSON, err := p.meta.RetrieveAttribute(nil, fi.Name(), "", aclkey) if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { return buckets, fmt.Errorf("get acl tag: %w", err) } - var acl auth.ACL - if len(aclTag) > 0 { - err = json.Unmarshal(aclTag, &acl) - if err != nil { - return buckets, fmt.Errorf("parse acl tag: %w", err) - } + acl, err := auth.ParseACL(aclJSON) + if err != nil { + return buckets, fmt.Errorf("parse acl tag: %w", err) } buckets = append(buckets, s3response.Bucket{ diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index 7fbaf53..2ef29df 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -1511,10 +1511,11 @@ func (s *S3Proxy) GetObjectLegalHold(ctx context.Context, bucket, object, versio } func (s *S3Proxy) ChangeBucketOwner(ctx context.Context, bucket string, acl []byte) error { - var acll auth.ACL - if err := json.Unmarshal(acl, &acll); err != nil { - return fmt.Errorf("unmarshal acl: %w", err) + acll, err := auth.ParseACL(acl) + if err != nil { + return err } + req, err := http.NewRequest(http.MethodPatch, fmt.Sprintf("%v/change-bucket-owner/?bucket=%v&owner=%v", s.endpoint, bucket, acll.Owner), nil) if err != nil { return fmt.Errorf("failed to send the request: %w", err)