Commit Graph
3 Commits
Author SHA1 Message Date
Chris Lu 8d59069a0a s3: return BucketAlreadyOwnedByYou when recreating your own bucket (#9822)
* s3: return BucketAlreadyOwnedByYou when recreating your own bucket

PutBucket returned BucketAlreadyExists for every existing bucket, even
when the caller already owns it, so idempotent re-creation (e.g. a
container that creates its bucket on startup) couldn't tell "someone
else took the name" from "it's already mine".

Recreating a bucket you own now returns BucketAlreadyOwnedByYou, unless
the request conflicts with the existing bucket: a different Object Lock
setting, or an ACL on the request or the existing bucket. To detect the
latter, a requested non-default canned/grant ACL is now persisted on
creation instead of being dropped.

* s3: fail PutBucket when the existing bucket's config can't be read

When a bucket already exists, an unreadable config left the recreate
defaulting to BucketAlreadyOwnedByYou, masking the backend error and
possibly accepting a conflicting recreate (Object Lock / ACL unknown).
Surface the read error instead.

* s3: return the stored bucket ACL from GetBucketAcl

GetBucketAcl always returned the owner's default full-control grant and
ignored any stored ACL, so a bucket created with a canned ACL or one set
via PutBucketAcl never read back correctly. Decode the stored grants
instead, sharing one grants-to-XML helper with the object ACL handler.

The shared helper also emits each grantee's real xsi:type (e.g. Group for
public-read) instead of a hardcoded CanonicalUser, so group grants read
back correctly for both bucket and object ACLs.

* s3: resolve the right already-exists error on the concurrent-create race

When two requests create the same bucket at once, the loser's mkdir
fails and the handler fell back to a flat BucketAlreadyExists, bypassing
the same-owner idempotency check. Route both the pre-check and the race
fallback through one existingBucketError helper so a same-owner recreate
still gets BucketAlreadyOwnedByYou.

* s3: record the bucket owner's account id at creation

setBucketOwner only stored the creating identity name, so the canonical
account id wasn't available later. Persist it under ExtAmzOwnerKey too,
the same field PutBucketAcl writes, so the bucket owner can be reported
independently of whoever reads it.

* s3: report the bucket owner from GetBucketAcl, not the caller

GetBucketAcl built the ACL Owner from the caller's account header, so an
admin or cross-account read returned the wrong owner. Use the owner
persisted on the bucket, falling back to the caller only when none is
recorded.
2026-06-04 15:33:03 -07:00
7y-9 bbbc3925ec fix: validate s3 ownership controls rule (#9684) 2026-05-27 14:41:10 -07:00
Chris Lu 37b6a14b0d feat(s3): add four bucket configuration handlers (#9570)
* feat(s3): add four bucket configuration handlers

- GetBucketPolicyStatus: computes IsPublic from the existing bucket policy
- PutBucketRequestPayment: companion writer to the existing GET; accepts
  only BucketOwner
- GetBucketAccelerateConfiguration: returns <Status>Suspended</Status>
- GetBucketLogging: returns an empty BucketLoggingStatus

Lets AWS SDK probes succeed instead of returning MethodNotAllowed.

* review: route GetBucketPolicyStatus through checkBucket

Mirrors the existence/auth gating used by other bucket handlers and
drops the bespoke filer_pb lookup so NoSuchBucket precedence is
consistent across the API surface.

* review: cap PutBucketRequestPayment body with MaxBytesReader

The body is unmarshalled as RequestPaymentConfiguration, which is a
handful of bytes; reject excessively large payloads up front and
defer Close immediately after wrapping.

* review: gate static getters on checkBucket

GetBucketAccelerateConfiguration and GetBucketLogging now run the
standard bucket existence check before returning the static
Suspended / empty-status response so a missing bucket cannot appear
to have valid configuration.

* review: share cache helper across misc tests; check io.ReadAll error

Accelerate and Logging tests now run through newMiscTestServer like
the others so the checkBucket guard sees a cached bucket; the
ReadAll error is explicitly checked.
2026-05-19 17:35:08 -07:00