fix: match S3 for conditional deletes of missing keys, delete markers and versions

A conditional delete of a key with no current version used to succeed. `DeleteObject` with `If-Match`, and a `DeleteObjects` key with an `ETag`, reported success for a missing key. When the current version was a delete marker, the condition was checked against the ETag the marker kept from the version it hides, so it could pass and create a second marker. S3 fails both with `NoSuchKey` and creates no marker. The posix backend now checks the conditions against the current version in `checkDeletePreconditions`, before its missing-key shortcuts. The azure backend returns `NoSuchKey` for a missing blob instead of skipping the conditions. `ObjectDeletePreconditions.IsSet` reports whether a delete has any condition.

A condition only applies to the current version. `DeleteObject` with a `versionId` and `If-Match` now returns `NotImplemented` with `<Header>If-Match</Header>` and the S3 additional message, ahead of authorization. Before, it checked the condition against that version and deleted the version. In `DeleteObjects`, an empty `<ETag>` fails its key with `InvalidArgument` and a `VersionId` together with an `ETag` fails it with `NotImplemented`, both ahead of the key's authorization.

The `PreconditionFailed` and `NotImplemented` messages drop their trailing period, which S3 doesn't use on any operation.
This commit is contained in:
niksis02
2026-10-10 11:43:19 +01:00
parent d7895e35e4
commit bca801bf3a
18 changed files with 797 additions and 86 deletions
+24 -27
View File
@@ -1111,39 +1111,36 @@ func (az *Azure) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input
}
func (az *Azure) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
if input.IfMatch != nil || input.IfMatchLastModifiedTime != nil || input.IfMatchSize != nil {
// evaluate the preconditions before deleting the object
conds := backend.ObjectDeletePreconditions{
IfMatch: input.IfMatch,
IfMatchLastModTime: input.IfMatchLastModifiedTime,
IfMatchSize: input.IfMatchSize,
}
if conds.IsSet() {
// evaluate the preconditions before deleting the object, a
// missing object fails them with NoSuchKey
props, err := az.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: input.Bucket,
Key: input.Key,
})
if err != nil && !errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey)) {
// if object doesn't exist, skip preconditions
// if unexpected error shows up, return the error
if err != nil {
return nil, err
}
if err == nil {
var etag string
if props.ETag != nil {
etag = *props.ETag
}
var lastMod time.Time
if props.LastModified != nil {
lastMod = *props.LastModified
}
var size int64
if props.ContentLength != nil {
size = *props.ContentLength
}
err := backend.EvaluateObjectDeletePreconditions(etag, lastMod, size,
backend.ObjectDeletePreconditions{
IfMatch: input.IfMatch,
IfMatchLastModTime: input.IfMatchLastModifiedTime,
IfMatchSize: input.IfMatchSize,
})
if err != nil {
return nil, err
}
var etag string
if props.ETag != nil {
etag = *props.ETag
}
var lastMod time.Time
if props.LastModified != nil {
lastMod = *props.LastModified
}
var size int64
if props.ContentLength != nil {
size = *props.ContentLength
}
err = backend.EvaluateObjectDeletePreconditions(etag, lastMod, size, conds)
if err != nil {
return nil, err
}
}
+9 -3
View File
@@ -851,9 +851,15 @@ type ObjectDeletePreconditions struct {
IfMatchSize *int64
}
// EvaluateObjectDeletePreconditions evaluates preconditions for DeleteObject
// and for each object in DeleteObjects. An If-Match (or ObjectIdentifier ETag)
// of "*" matches any ETag.
// IsSet reports whether the delete has any condition. A conditional delete
// of a key with no current version, including one whose current version is
// a delete marker, fails with NoSuchKey instead of succeeding.
func (p ObjectDeletePreconditions) IsSet() bool {
return p.IfMatch != nil || p.IfMatchLastModTime != nil || p.IfMatchSize != nil
}
// EvaluateObjectDeletePreconditions evaluates the DeleteObject conditions
// against an existing object. If-Match "*" matches any object.
func EvaluateObjectDeletePreconditions(etag string, modTime time.Time, size int64, preconditions ObjectDeletePreconditions) error {
etag = strings.Trim(etag, `"`)
if preconditions.IfMatch != nil {
+63 -26
View File
@@ -4480,6 +4480,44 @@ func (p *Posix) checkPutPreconditions(bucket, object string, ifMatch, ifNoneMatc
return nil
}
// checkDeletePreconditions evaluates the conditions of a delete of the
// current version of key. A key with no current version, including one
// whose current version is a delete marker, fails them with NoSuchKey.
func (p *Posix) checkDeletePreconditions(bucket, object string, conds backend.ObjectDeletePreconditions) error {
if !conds.IsSet() {
return nil
}
// the entry at the object path may be the object of the key with or
// without the trailing slash
fi, err := p.statLiveObject(bucket, object)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if isErrNameTooLong(err) {
return s3err.GetKeyTooLongErr(int64(len(object)), 1024)
}
if err != nil {
return fmt.Errorf("stat object: %w", err)
}
// a delete marker keeps the etag of the version it hides
isDel, err := p.isObjDeleteMarker(bucket, object)
if err != nil {
return err
}
if isDel {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
etag, err := p.meta.RetrieveAttribute(nil, bucket, object, etagkey)
if err != nil || len(etag) == 0 {
etag = []byte(p.defaultEtag)
}
return backend.EvaluateObjectDeletePreconditions(string(etag), fi.ModTime(), fi.Size(), conds)
}
// snapshotObjVersion copies the current object at bucket/key into the
// versioning directory before the object is replaced, if versioning is
// configured for the bucket and there is an object to snapshot.
@@ -5165,13 +5203,18 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return nil, err
}
evalPreconditions := func(f os.FileInfo, bucket, object string) error {
var err error
if f == nil {
f, err = os.Stat(p.ObjectPath(bucket, object))
if err != nil {
return nil
}
conds := backend.ObjectDeletePreconditions{
IfMatch: input.IfMatch,
IfMatchLastModTime: input.IfMatchLastModifiedTime,
IfMatchSize: input.IfMatchSize,
}
// evalVersionPreconditions evaluates the conditions against the
// version at bucket/object
evalVersionPreconditions := func(bucket, object string) error {
f, err := os.Stat(p.ObjectPath(bucket, object))
if err != nil {
return nil
}
b, err := p.meta.RetrieveAttribute(nil, bucket, object, etagkey)
@@ -5180,17 +5223,16 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
etag = p.defaultEtag
}
// evaluate preconditions
return backend.EvaluateObjectDeletePreconditions(etag, f.ModTime(), f.Size(),
backend.ObjectDeletePreconditions{
IfMatch: input.IfMatch,
IfMatchLastModTime: input.IfMatchLastModifiedTime,
IfMatchSize: input.IfMatchSize,
})
return backend.EvaluateObjectDeletePreconditions(etag, f.ModTime(), f.Size(), conds)
}
if p.versioningEnabled() && vStatus != "" {
if getString(input.VersionId) == "" {
err := p.checkDeletePreconditions(bucket, object, conds)
if err != nil {
return nil, err
}
// if the versionId is not specified, make the current version a delete marker
fi, err := os.Stat(objpath)
if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) {
@@ -5214,11 +5256,6 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return &s3.DeleteObjectOutput{}, nil
}
err = evalPreconditions(fi, bucket, object)
if err != nil {
return nil, err
}
acct, ok := ctx.Value("account").(auth.Account)
if !ok {
acct = auth.Account{}
@@ -5306,7 +5343,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
if string(vId) == *input.VersionId {
// evaluate preconditions
err := evalPreconditions(nil, bucket, object)
err := evalVersionPreconditions(bucket, object)
if err != nil {
return nil, err
}
@@ -5443,7 +5480,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
}, nil
}
err = evalPreconditions(nil, versionPath, *input.VersionId)
err = evalVersionPreconditions(versionPath, *input.VersionId)
if err != nil {
return nil, err
}
@@ -5478,6 +5515,11 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return &s3.DeleteObjectOutput{VersionId: input.VersionId}, nil
}
err = p.checkDeletePreconditions(bucket, object, conds)
if err != nil {
return nil, err
}
fi, err := os.Stat(objpath)
if isErrNameTooLong(err) {
return nil, s3err.GetKeyTooLongErr(int64(len(object)), 1024)
@@ -5503,11 +5545,6 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return &s3.DeleteObjectOutput{}, nil
}
err = evalPreconditions(fi, bucket, object)
if err != nil {
return nil, err
}
err = os.Remove(objpath)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
@@ -0,0 +1,188 @@
// 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"
"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/stretchr/testify/assert"
"github.com/versity/versitygw/backend/meta"
"github.com/versity/versitygw/s3err"
)
func TestPosixConditionalDelete(t *testing.T) {
for name, mkMeta := range metaModes(t) {
t.Run(name, func(t *testing.T) {
ctx := context.Background()
p := newVersionedTestPosix(t, mkMeta)
bucket, vBucket := "bucket", "versioned"
createTestBucket(t, p, bucket)
createTestBucket(t, p, vBucket)
err := p.PutBucketVersioning(ctx, vBucket, types.BucketVersioningStatusEnabled)
if err != nil {
t.Fatalf("put bucket versioning: %v", err)
}
del := func(bucket, key string, ifMatch *string) (*s3.DeleteObjectOutput, error) {
return p.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &key,
IfMatch: ifMatch,
})
}
t.Run("stale etag keeps the object", func(t *testing.T) {
for _, b := range []string{bucket, vBucket} {
key := "stale"
if _, err := testPut(p, b, key, []byte("data"), nil, nil); err != nil {
t.Fatalf("put object: %v", err)
}
_, err := del(b, key, aws.String("00000000000000000000000000000000"))
if !isPreconditionFailed(err) {
t.Fatalf("%v: expected PreconditionFailed, got %v", b, err)
}
getTestObject(t, p, b, key)
}
})
t.Run("matching etag deletes the object", func(t *testing.T) {
for _, b := range []string{bucket, vBucket} {
for _, ifMatch := range []func(etag string) string{
trimEtag,
func(etag string) string { return etag },
func(string) string { return "*" },
func(string) string { return `"*"` },
} {
key := "match"
res, err := testPut(p, b, key, []byte("data"), nil, nil)
if err != nil {
t.Fatalf("put object: %v", err)
}
out, err := del(b, key, aws.String(ifMatch(res.ETag)))
if err != nil {
t.Fatalf("%v: delete with If-Match %q: %v", b, ifMatch(res.ETag), err)
}
if b == vBucket {
assert.True(t, aws.ToBool(out.DeleteMarker))
}
_, err = p.HeadObject(ctx, &s3.HeadObjectInput{Bucket: &b, Key: &key})
assert.ErrorIs(t, err, s3err.GetAPIError(s3err.ErrNoSuchKey))
}
}
})
t.Run("missing key", func(t *testing.T) {
for _, b := range []string{bucket, vBucket} {
for _, ifMatch := range []string{"*", "00000000000000000000000000000000"} {
_, err := del(b, "missing", &ifMatch)
assert.ErrorIs(t, err, s3err.GetAPIError(s3err.ErrNoSuchKey))
}
}
})
t.Run("current delete marker", func(t *testing.T) {
key := "marked"
res, err := testPut(p, vBucket, key, []byte("data"), nil, nil)
if err != nil {
t.Fatalf("put object: %v", err)
}
if _, err := del(vBucket, key, nil); err != nil {
t.Fatalf("create delete marker: %v", err)
}
// the marker keeps the etag of the version it hides
for _, ifMatch := range []string{"*", trimEtag(res.ETag)} {
_, err := del(vBucket, key, &ifMatch)
assert.ErrorIs(t, err, s3err.GetAPIError(s3err.ErrNoSuchKey))
}
out, err := p.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
Bucket: &vBucket,
Prefix: &key,
MaxKeys: aws.Int32(1000),
})
if err != nil {
t.Fatalf("list object versions: %v", err)
}
assert.Len(t, out.DeleteMarkers, 1)
assert.Len(t, out.Versions, 1)
})
t.Run("delete objects evaluates each etag", func(t *testing.T) {
keys := []string{"batch-match", "batch-stale", "batch-wildcard"}
etags := map[string]string{}
for _, key := range keys {
res, err := testPut(p, bucket, key, []byte(key), nil, nil)
if err != nil {
t.Fatalf("put object: %v", err)
}
etags[key] = res.ETag
}
out, err := p.DeleteObjects(ctx, &s3.DeleteObjectsInput{
Bucket: &bucket,
Delete: &types.Delete{
Objects: []types.ObjectIdentifier{
{Key: aws.String("batch-match"), ETag: aws.String(etags["batch-match"])},
{Key: aws.String("batch-stale"), ETag: aws.String(trimEtag(etags["batch-match"]))},
{Key: aws.String("batch-wildcard"), ETag: aws.String("*")},
{Key: aws.String("batch-missing"), ETag: aws.String("*")},
},
},
})
if err != nil {
t.Fatalf("delete objects: %v", err)
}
var deleted []string
for _, d := range out.Deleted {
deleted = append(deleted, aws.ToString(d.Key))
}
assert.Equal(t, []string{"batch-match", "batch-wildcard"}, deleted)
errCodes := map[string]string{}
for _, e := range out.Error {
errCodes[aws.ToString(e.Key)] = aws.ToString(e.Code)
}
assert.Equal(t, map[string]string{
"batch-stale": "PreconditionFailed",
"batch-missing": "NoSuchKey",
}, errCodes)
data, _ := getTestObject(t, p, bucket, "batch-stale")
assert.Equal(t, "batch-stale", string(data))
})
})
}
}
// newVersionedTestPosix creates a Posix backend with a versioning directory,
// so that versioning can be enabled on its buckets
func newVersionedTestPosix(t *testing.T, mkMeta func(t *testing.T) (meta.MetadataStorer, PosixOpts)) *Posix {
t.Helper()
storer, opts := mkMeta(t)
opts.VersioningDir = t.TempDir()
p, err := New(t.TempDir(), storer, opts)
if err != nil {
t.Fatalf("new posix: %v", err)
}
return p
}
+11
View File
@@ -125,6 +125,17 @@ func (c S3ApiController) DeleteObjects(ctx fiber.Ctx) (*Response, error) {
toDelete := make([]types.ObjectIdentifier, 0, len(dObj.Objects))
for i, obj := range dObj.Objects {
// The ETag condition of an object is validated ahead of its
// authorization. It only applies to the current version, so it
// can't be combined with a version id.
if obj.ETag != nil {
switch {
case *obj.ETag == "":
checkErrs[i] = s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyETag, "")
case backend.GetStringFromPtr(obj.VersionId) != "":
checkErrs[i] = s3err.GetAPIError(s3err.ErrNotImplementedFormField)
}
}
if checkErrs[i] == nil {
toDelete = append(toDelete, obj)
}
+65
View File
@@ -113,6 +113,19 @@ func TestS3ApiController_DeleteObjects(t *testing.T) {
lockConfig, err := json.Marshal(auth.BucketLockConfig{Enabled: true})
assert.NoError(t, err)
emptyETag := ""
etagConditionBody, err := xml.Marshal(s3response.DeleteObjects{
Objects: []types.ObjectIdentifier{
{Key: utils.GetStringPtr("empty"), ETag: &emptyETag},
{Key: utils.GetStringPtr("versioned"), VersionId: utils.GetStringPtr("v1"), ETag: utils.GetStringPtr("etag")},
{Key: utils.GetStringPtr("ok"), ETag: utils.GetStringPtr("etag")},
},
})
assert.NoError(t, err)
emptyETagCode, emptyETagMessage := "InvalidArgument", "The value provided for the ETag field cannot be empty for this API."
versionedETagCode, versionedETagMessage := "NotImplemented", "A form field you provided implies functionality that is not implemented"
legalHoldOn, legalHoldOff := true, false
lockedObjectCode := "AccessDenied"
lockedObjectMessage := "Access Denied because object protected by object lock."
@@ -392,6 +405,58 @@ func TestS3ApiController_DeleteObjects(t *testing.T) {
}
},
},
{
name: "ETag conditions validated ahead of authorization",
input: testInput{
locals: defaultLocals,
body: etagConditionBody,
},
output: testOutput{
response: &Response{
Data: s3response.DeleteResult{
Deleted: []types.DeletedObject{
{Key: utils.GetStringPtr("ok")},
},
Error: []types.Error{
{Key: utils.GetStringPtr("empty"), Code: &emptyETagCode, Message: &emptyETagMessage},
{Key: utils.GetStringPtr("versioned"), VersionId: utils.GetStringPtr("v1"), Code: &versionedETagCode, Message: &versionedETagMessage},
},
},
MetaOpts: &MetaOptions{
BucketOwner: "root",
EventName: s3event.EventObjectRemovedDeleteObjects,
ObjectCount: 3,
},
},
},
configureMock: func(be *BackendMock) {
// every object but "ok" is locked, which the ETag
// errors take precedence over
be.GetObjectLockConfigurationFunc = func(contextMoqParam context.Context, bucket string) ([]byte, error) {
return lockConfig, nil
}
be.GetBucketVersioningFunc = func(contextMoqParam context.Context, bucket string) (s3response.GetBucketVersioningOutput, error) {
return s3response.GetBucketVersioningOutput{}, nil
}
be.GetObjectRetentionFunc = func(contextMoqParam context.Context, bucket, object, versionId string) ([]byte, error) {
return []byte("{}"), nil
}
be.GetObjectLegalHoldFunc = func(contextMoqParam context.Context, bucket, object, versionId string) (*bool, error) {
if object == "ok" {
return &legalHoldOff, nil
}
return &legalHoldOn, nil
}
be.DeleteObjectsFunc = func(contextMoqParam context.Context, deleteObjectsInput *s3.DeleteObjectsInput) (s3response.DeleteResult, error) {
assert.Len(t, deleteObjectsInput.Delete.Objects, 1)
assert.Equal(t, "ok", *deleteObjectsInput.Delete.Objects[0].Key)
assert.Equal(t, "etag", *deleteObjectsInput.Delete.Objects[0].ETag)
return s3response.DeleteResult{
Deleted: []types.DeletedObject{{Key: utils.GetStringPtr("ok")}},
}, nil
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
+13
View File
@@ -23,7 +23,9 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/gofiber/fiber/v3"
"github.com/versity/versitygw/auth"
"github.com/versity/versitygw/debuglogger"
"github.com/versity/versitygw/s3api/utils"
"github.com/versity/versitygw/s3err"
"github.com/versity/versitygw/s3event"
)
@@ -147,6 +149,17 @@ func (c S3ApiController) DeleteObject(ctx fiber.Ctx) (*Response, error) {
}, err
}
// If-Match only applies to the current version of the object, and is
// rejected ahead of authorization when a version is named
if versionId != "" && ifMatch != nil {
debuglogger.Logf("conditional delete with versionId %q", versionId)
return &Response{
MetaOpts: &MetaOptions{
BucketOwner: parsedAcl.Owner,
},
}, s3err.GetNotImplementedErr("If-Match", s3err.NmpAdditionalMessageVersionedDelete)
}
action := auth.DeleteObjectAction
if versionId != "" {
action = auth.DeleteObjectVersionAction
+21
View File
@@ -241,6 +241,26 @@ func TestS3ApiController_DeleteObject(t *testing.T) {
err: s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyVersionId, ""),
},
},
{
name: "If-Match with a versionId is rejected ahead of authorization",
input: testInput{
locals: accessDeniedLocals,
queries: map[string]string{
"versionId": "versionId",
},
headers: map[string]string{
"If-Match": "etag",
},
},
output: testOutput{
response: &Response{
MetaOpts: &MetaOptions{
BucketOwner: "root",
},
},
err: s3err.GetNotImplementedErr("If-Match", s3err.NmpAdditionalMessageVersionedDelete),
},
},
{
name: "verify access fails",
input: testInput{
@@ -340,6 +360,7 @@ func TestS3ApiController_DeleteObject(t *testing.T) {
ctxInputs{
locals: tt.input.locals,
queries: tt.input.queries,
headers: tt.input.headers,
})
})
}
+5
View File
@@ -75,6 +75,7 @@ const (
InvalidArgSSECRequiresTLS
InvalidArgUploadIdWithUploads
InvalidArgBucketRegionEndpoint
InvalidArgEmptyETag
)
var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
@@ -206,6 +207,10 @@ var invalidArgErrResponses = map[InvalidArgErrorCode]InvalidArgumentError{
Description: "Version id cannot be the empty string",
ArgumentName: "versionId",
},
InvalidArgEmptyETag: {
Description: "The value provided for the ETag field cannot be empty for this API.",
ArgumentName: "ETag",
},
InvalidArgChecksumPart: {
Description: "Invalid Base64 or multiple checksums present in request",
ArgumentName: "Checksum",
+1
View File
@@ -24,6 +24,7 @@ const (
NmpAdditionalMessageIfMatch NmpAdditionalMessage = "We don't accept the provided value of If-Match header for this API"
NmpAdditionalMessageIfNoneMatch NmpAdditionalMessage = "We don't accept the provided value of If-None-Match header for this API"
NmpAdditionalMessageMultipleCondHeaders NmpAdditionalMessage = "Multiple conditional request headers present in the request"
NmpAdditionalMessageVersionedDelete NmpAdditionalMessage = "Conditional delete operations are not allowed when a version ID is included in the request parameters."
)
// NotImplementedError is returned when a header implies unsupported functionality.
+8 -2
View File
@@ -145,6 +145,7 @@ const (
ErrInvalidRequest
ErrAuthNotSetup
ErrNotImplemented
ErrNotImplementedFormField
ErrPreconditionFailed
ErrInvalidObjectState
ErrInvalidRange
@@ -484,12 +485,17 @@ var errorCodeResponse = map[ErrorCode]APIError{
},
ErrNotImplemented: {
Code: "NotImplemented",
Description: "A header you provided implies functionality that is not implemented.",
Description: "A header you provided implies functionality that is not implemented",
HTTPStatusCode: http.StatusNotImplemented,
},
ErrNotImplementedFormField: {
Code: "NotImplemented",
Description: "A form field you provided implies functionality that is not implemented",
HTTPStatusCode: http.StatusNotImplemented,
},
ErrPreconditionFailed: {
Code: "PreconditionFailed",
Description: "At least one of the pre-conditions you specified did not hold.",
Description: "At least one of the pre-conditions you specified did not hold",
HTTPStatusCode: http.StatusPreconditionFailed,
},
ErrInvalidObjectState: {
+5 -3
View File
@@ -2048,7 +2048,9 @@ func AccessControl_bucket_policy_condition_if_match_versioned_delete(s *S3Conf)
}
// "null" is the version id every object carries until versioning is
// enabled, so this is a versioned delete on any backend.
// enabled, so this is a versioned delete on any backend. A versioned
// delete carrying If-Match is rejected before the policy is
// evaluated.
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
@@ -2057,8 +2059,8 @@ func AccessControl_bucket_policy_condition_if_match_versioned_delete(s *S3Conf)
IfMatch: &etag,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
return fmt.Errorf("a versioned delete must leave s3:if-match absent: %w", err)
if err := checkApiErr(err, s3err.GetNotImplementedErr("If-Match", s3err.NmpAdditionalMessageVersionedDelete)); err != nil {
return fmt.Errorf("a versioned delete carrying If-Match must be rejected: %w", err)
}
// The same header on the same object, minus the version, is a plain
+89
View File
@@ -220,6 +220,95 @@ func DeleteObject_conditional_writes(s *S3Conf) error {
})
}
func DeleteObject_conditional_wildcard(s *S3Conf) error {
testName := "DeleteObject_conditional_wildcard"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
for _, ifMatch := range []string{"*", `"*"`} {
obj := "my-obj"
_, err := putObjects(s3client, []string{obj}, bucket)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
IfMatch: &ifMatch,
})
cancel()
if err != nil {
return fmt.Errorf("If-Match %v: %w", ifMatch, err)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err := checkSdkApiErr(err, "NotFound"); err != nil {
return fmt.Errorf("If-Match %v: %w", ifMatch, err)
}
}
return nil
})
}
func DeleteObject_conditional_non_existing_object(s *S3Conf) error {
testName := "DeleteObject_conditional_non_existing_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
for _, ifMatch := range []string{"*", "d41d8cd98f00b204e9800998ecf8427e"} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("my-obj"),
IfMatch: &ifMatch,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil {
return fmt.Errorf("If-Match %v: %w", ifMatch, err)
}
}
return nil
})
}
func DeleteObject_conditional_with_version_id(s *S3Conf) error {
testName := "DeleteObject_conditional_with_version_id"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
res, err := putObjects(s3client, []string{obj}, bucket)
if err != nil {
return err
}
for _, ifMatch := range []*string{res[0].ETag, getPtr("*")} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
VersionId: getPtr("null"),
IfMatch: ifMatch,
})
cancel()
if err := checkApiErr(err, s3err.GetNotImplementedErr("If-Match", s3err.NmpAdditionalMessageVersionedDelete)); err != nil {
return fmt.Errorf("If-Match %v: %w", *ifMatch, err)
}
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
return err
})
}
func DeleteObject_directory_not_empty(s *S3Conf) error {
testName := "DeleteObject_directory_not_empty"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+104
View File
@@ -17,6 +17,8 @@ package integration
import (
"context"
"fmt"
"slices"
"strings"
"github.com/aws/aws-sdk-go-v2/service/iam"
"github.com/aws/aws-sdk-go-v2/service/s3"
@@ -217,6 +219,108 @@ func DeleteObjects_quiet_mode(s *S3Conf) error {
})
}
// DeleteObjects_conditional checks that the ETag of each object is a
// condition of its delete: a stale ETag keeps the object, a missing key
// fails with NoSuchKey, and an empty ETag or one with a version id is
// rejected. The checks don't depend on the order of the results.
func DeleteObjects_conditional(s *S3Conf) error {
testName := "DeleteObjects_conditional"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
deleted := []string{"quoted", "unquoted", "wildcard", "quoted-wildcard"}
kept := []string{"stale", "empty", "versioned"}
contents, err := putObjects(s3client, append(deleted, kept...), bucket)
if err != nil {
return err
}
etags := map[string]string{}
for _, obj := range contents {
etags[getString(obj.Key)] = getString(obj.ETag)
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
Bucket: &bucket,
Delete: &types.Delete{
Objects: []types.ObjectIdentifier{
{Key: getPtr("quoted"), ETag: getPtr(etags["quoted"])},
{Key: getPtr("unquoted"), ETag: getPtr(strings.Trim(etags["unquoted"], `"`))},
{Key: getPtr("wildcard"), ETag: getPtr("*")},
{Key: getPtr("quoted-wildcard"), ETag: getPtr(`"*"`)},
{Key: getPtr("stale"), ETag: getPtr("00000000000000000000000000000000")},
{Key: getPtr("empty"), ETag: getPtr("")},
{Key: getPtr("versioned"), VersionId: getPtr("null"), ETag: getPtr(etags["versioned"])},
{Key: getPtr("missing"), ETag: getPtr("*")},
{Key: getPtr("missing-etag"), ETag: getPtr(etags["quoted"])},
},
},
})
cancel()
if err != nil {
return err
}
gotDeleted := map[string]bool{}
for _, obj := range out.Deleted {
gotDeleted[getString(obj.Key)] = true
}
if len(gotDeleted) != len(deleted) {
return fmt.Errorf("expected the deleted objects to be %v, instead got %v",
deleted, gotDeleted)
}
for _, key := range deleted {
if !gotDeleted[key] {
return fmt.Errorf("expected %v to be deleted, instead got %v", key, gotDeleted)
}
}
wantErrs := map[string]s3err.S3Error{
"stale": s3err.GetAPIError(s3err.ErrPreconditionFailed),
"empty": s3err.GetInvalidArgumentErr(s3err.InvalidArgEmptyETag, ""),
"versioned": s3err.GetAPIError(s3err.ErrNotImplementedFormField),
"missing": s3err.GetAPIError(s3err.ErrNoSuchKey),
"missing-etag": s3err.GetAPIError(s3err.ErrNoSuchKey),
}
if len(out.Errors) != len(wantErrs) {
return fmt.Errorf("expected %v errors, instead got %v", len(wantErrs), out.Errors)
}
for _, e := range out.Errors {
key := getString(e.Key)
wantErr, ok := wantErrs[key]
if !ok {
return fmt.Errorf("unexpected error for %v: %v", key, getString(e.Code))
}
if err := checkDeleteObjectsErr(e, key, wantErr); err != nil {
return err
}
if key == "versioned" && getString(e.VersionId) != "null" {
return fmt.Errorf("expected the versioned error to name version null, instead got %v",
getString(e.VersionId))
}
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
var gotKept []string
for _, obj := range res.Contents {
gotKept = append(gotKept, getString(obj.Key))
}
slices.Sort(kept)
if !slices.Equal(gotKept, kept) {
return fmt.Errorf("expected the remaining objects to be %v, instead got %v",
kept, gotKept)
}
return nil
})
}
// DeleteObjects_iam_mixed_denials_and_success covers a single batch mixing
// every DeleteObjects outcome at once: a key the identity policy denies, a
// governance-locked key with no bypass, and keys the caller may freely
+12
View File
@@ -372,6 +372,9 @@ func TestDeleteObject(ts *TestState) {
ts.Run(DeleteObject_directory_object)
ts.Run(DeleteObject_non_empty_dir_obj)
ts.Run(DeleteObject_conditional_writes)
ts.Run(DeleteObject_conditional_wildcard)
ts.Run(DeleteObject_conditional_non_existing_object)
ts.Run(DeleteObject_conditional_with_version_id)
ts.Run(DeleteObject_success)
ts.Run(DeleteObject_success_status_code)
ts.Run(DeleteObject_incorrect_expected_bucket_owner)
@@ -385,6 +388,7 @@ func TestDeleteObjects(ts *TestState) {
ts.Run(DeleteObjects_non_existing_objects)
ts.Run(DeleteObjects_success)
ts.Run(DeleteObjects_quiet_mode)
ts.Run(DeleteObjects_conditional)
ts.Run(DeleteObjects_key_limit)
ts.Run(DeleteObjects_invalid_object_keys)
}
@@ -2143,9 +2147,11 @@ func TestVersioning(ts *TestState) {
ts.Run(Versioning_DeleteObject_non_existing_objects)
ts.Run(Versioning_DeleteObject_suspended)
ts.Run(Versioning_DeleteObject_never_versioned_bucket)
ts.Run(Versioning_DeleteObject_conditional_delete_marker)
ts.Run(Versioning_DeleteObjects_success)
ts.Run(Versioning_DeleteObjects_delete_deleteMarkers)
ts.Run(Versioning_DeleteObjects_never_versioned_bucket)
ts.Run(Versioning_DeleteObjects_conditional)
// ListObjectVersions
ts.Run(ListObjectVersions_non_existing_bucket)
ts.Run(ListObjectVersions_negative_max_keys)
@@ -3191,6 +3197,9 @@ func GetIntTests() IntTests {
"DeleteObject_directory_object_noslash": DeleteObject_directory_object_noslash,
"DeleteObject_non_empty_dir_obj": DeleteObject_non_empty_dir_obj,
"DeleteObject_conditional_writes": DeleteObject_conditional_writes,
"DeleteObject_conditional_wildcard": DeleteObject_conditional_wildcard,
"DeleteObject_conditional_non_existing_object": DeleteObject_conditional_non_existing_object,
"DeleteObject_conditional_with_version_id": DeleteObject_conditional_with_version_id,
"DeleteObject_name_too_long": DeleteObject_name_too_long,
"CopyObject_overwrite_same_dir_object": CopyObject_overwrite_same_dir_object,
"CopyObject_overwrite_same_file_object": CopyObject_overwrite_same_file_object,
@@ -3210,6 +3219,7 @@ func GetIntTests() IntTests {
"DeleteObjects_non_existing_objects": DeleteObjects_non_existing_objects,
"DeleteObjects_success": DeleteObjects_success,
"DeleteObjects_quiet_mode": DeleteObjects_quiet_mode,
"DeleteObjects_conditional": DeleteObjects_conditional,
"DeleteObjects_key_limit": DeleteObjects_key_limit,
"DeleteObjects_invalid_object_keys": DeleteObjects_invalid_object_keys,
"DeleteObjects_iam_mixed_denials_and_success": DeleteObjects_iam_mixed_denials_and_success,
@@ -3810,9 +3820,11 @@ func GetIntTests() IntTests {
"Versioning_DeleteObject_non_existing_objects": Versioning_DeleteObject_non_existing_objects,
"Versioning_DeleteObject_suspended": Versioning_DeleteObject_suspended,
"Versioning_DeleteObject_never_versioned_bucket": Versioning_DeleteObject_never_versioned_bucket,
"Versioning_DeleteObject_conditional_delete_marker": Versioning_DeleteObject_conditional_delete_marker,
"Versioning_DeleteObjects_success": Versioning_DeleteObjects_success,
"Versioning_DeleteObjects_delete_deleteMarkers": Versioning_DeleteObjects_delete_deleteMarkers,
"Versioning_DeleteObjects_never_versioned_bucket": Versioning_DeleteObjects_never_versioned_bucket,
"Versioning_DeleteObjects_conditional": Versioning_DeleteObjects_conditional,
"ListObjectVersions_non_existing_bucket": ListObjectVersions_non_existing_bucket,
"ListObjectVersions_negative_max_keys": ListObjectVersions_negative_max_keys,
"ListObjectVersions_list_single_object_versions": ListObjectVersions_list_single_object_versions,
+18 -24
View File
@@ -2952,12 +2952,9 @@ func S3IAMAccessControl_condition_if_match_delete_object(s *S3Conf) error {
})
}
// S3IAMAccessControl_condition_if_match_versioned_delete pins the edge of
// s3:if-match's action set on the identity-policy side, where nothing
// validates a Condition's key against the action it names. A delete naming
// a version is authorized as s3:DeleteObjectVersion, which the key doesn't
// apply to, so it stays absent and a statement demanding it can never be
// satisfied
// S3IAMAccessControl_condition_if_match_versioned_delete checks that a
// delete naming a version and carrying If-Match is rejected before the
// identity policy is evaluated, whether the policy would deny or allow it.
func S3IAMAccessControl_condition_if_match_versioned_delete(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_if_match_versioned_delete"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
@@ -2979,42 +2976,39 @@ func S3IAMAccessControl_condition_if_match_versioned_delete(s *S3Conf) error {
// "null" is the version id every object carries until versioning is
// enabled, so this is a versioned delete on any backend.
deleteVersion := func(ifMatch string) error {
deleteVersion := func() error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
_, err := user.client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
VersionId: getPtr("null"),
IfMatch: &ifMatch,
IfMatch: &etag,
})
return err
}
if err := checkApiErr(deleteVersion(etag),
wantImplicitDeny(user.arn, actS3DeleteObjectVersion, objectArn(bucket, "obj"))); err != nil {
return fmt.Errorf("a versioned delete must leave s3:if-match absent: %w", err)
wantErr := s3err.GetNotImplementedErr("If-Match", s3err.NmpAdditionalMessageVersionedDelete)
if err := checkApiErr(deleteVersion(), wantErr); err != nil {
return fmt.Errorf("a versioned delete the policy denies: %w", err)
}
// Absent, not merely different: the same request satisfies a
// statement requiring the key to be absent.
if err := putS3IAMUserPolicy(root, user, "p", policyDoc(accessStatement{
Effect: "Allow", Action: actS3DeleteObjectVersion, Resource: objectsArn(bucket),
Condition: cond("Null", "s3:if-match", "true"),
})); err != nil {
return err
}
if err := checkApiErr(deleteVersion(), wantErr); err != nil {
return fmt.Errorf("a versioned delete the policy allows: %w", err)
}
// Authorized now, and the header still decides the delete: a
// mismatch fails the precondition the policy had no say in.
if err := checkApiErr(deleteVersion("0123456789abcdef0123456789abcdef"),
s3err.GetAPIError(s3err.ErrPreconditionFailed)); err != nil {
return fmt.Errorf("the precondition itself must still be enforced: %w", err)
}
if err := deleteVersion(etag); err != nil {
return fmt.Errorf("a versioned delete naming the object's ETag must be allowed: %w", err)
}
return nil
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s.GetClient().HeadObject(ctx, &s3.HeadObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
})
cancel()
return err
})
}
+160
View File
@@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"io"
"maps"
"net/http"
"net/url"
"strings"
@@ -4038,6 +4039,72 @@ func Versioning_DeleteObject_never_versioned_bucket(s *S3Conf) error {
})
}
// Versioning_DeleteObject_conditional_delete_marker checks that a
// conditional delete of a key whose current version is a delete marker
// fails with NoSuchKey and creates no other marker, while If-Match "*"
// on a key with a current version creates one.
func Versioning_DeleteObject_conditional_delete_marker(s *S3Conf) error {
testName := "Versioning_DeleteObject_conditional_delete_marker"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
return forEachKey([]string{"my-obj", "my-dir/"}, func(obj string) error {
versions, err := createObjVersions(s3client, bucket, obj, 1)
if err != nil {
return err
}
if _, err := createDeleteMarker(s3client, bucket, obj); err != nil {
return err
}
// the marker hides the version the etag belongs to
for _, ifMatch := range []*string{versions[0].ETag, getPtr("*")} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
IfMatch: ifMatch,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchKey)); err != nil {
return fmt.Errorf("If-Match %v: %w", *ifMatch, err)
}
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
Bucket: &bucket,
Prefix: &obj,
})
cancel()
if err != nil {
return err
}
if len(res.DeleteMarkers) != 1 {
return fmt.Errorf("expected 1 delete marker, instead got %v", len(res.DeleteMarkers))
}
if _, err := createObjVersions(s3client, bucket, obj, 1); err != nil {
return err
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
IfMatch: getPtr("*"),
})
cancel()
if err != nil {
return err
}
if out.DeleteMarker == nil || !*out.DeleteMarker || getString(out.VersionId) == "" {
return fmt.Errorf("expected a delete marker to be created, instead got %v, %v",
out.DeleteMarker, getString(out.VersionId))
}
return nil
})
}, withVersioning(types.BucketVersioningStatusEnabled))
}
func Versioning_DeleteObjects_success(s *S3Conf) error {
testName := "Versioning_DeleteObjects_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -4389,6 +4456,99 @@ func Versioning_DeleteObjects_never_versioned_bucket(s *S3Conf) error {
})
}
// Versioning_DeleteObjects_conditional checks the ETag conditions of
// DeleteObjects in a versioned bucket. The checks don't depend on the order
// of the results.
func Versioning_DeleteObjects_conditional(s *S3Conf) error {
testName := "Versioning_DeleteObjects_conditional"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
versions := map[string]types.ObjectVersion{}
for _, obj := range []string{"live", "marked", "versioned", "stale"} {
v, err := createObjVersions(s3client, bucket, obj, 1)
if err != nil {
return err
}
versions[obj] = v[0]
}
if _, err := createDeleteMarker(s3client, bucket, "marked"); err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
out, err := s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
Bucket: &bucket,
Delete: &types.Delete{
Objects: []types.ObjectIdentifier{
{Key: getPtr("live"), ETag: versions["live"].ETag},
{Key: getPtr("marked"), ETag: getPtr("*")},
{Key: getPtr("missing"), ETag: getPtr("*")},
{Key: getPtr("versioned"), VersionId: versions["versioned"].VersionId, ETag: versions["versioned"].ETag},
{Key: getPtr("stale"), ETag: getPtr("00000000000000000000000000000000")},
},
},
})
cancel()
if err != nil {
return err
}
if len(out.Deleted) != 1 {
return fmt.Errorf("expected 1 deleted object, instead got %v", out.Deleted)
}
del := out.Deleted[0]
if getString(del.Key) != "live" || del.DeleteMarker == nil || !*del.DeleteMarker ||
getString(del.DeleteMarkerVersionId) == "" || del.VersionId != nil {
return fmt.Errorf("expected a delete marker to be created for live, instead got %+v", del)
}
wantErrs := map[string]s3err.S3Error{
"marked": s3err.GetAPIError(s3err.ErrNoSuchKey),
"missing": s3err.GetAPIError(s3err.ErrNoSuchKey),
"versioned": s3err.GetAPIError(s3err.ErrNotImplementedFormField),
"stale": s3err.GetAPIError(s3err.ErrPreconditionFailed),
}
if len(out.Errors) != len(wantErrs) {
return fmt.Errorf("expected %v errors, instead got %v", len(wantErrs), out.Errors)
}
for _, e := range out.Errors {
key := getString(e.Key)
wantErr, ok := wantErrs[key]
if !ok {
return fmt.Errorf("unexpected error for %v: %v", key, getString(e.Code))
}
if err := checkDeleteObjectsErr(e, key, wantErr); err != nil {
return err
}
if key == "versioned" && getString(e.VersionId) != getString(versions["versioned"].VersionId) {
return fmt.Errorf("expected the versioned error to name version %v, instead got %v",
getString(versions["versioned"].VersionId), getString(e.VersionId))
}
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}
markers := map[string]int{}
for _, dm := range res.DeleteMarkers {
markers[getString(dm.Key)]++
}
if !maps.Equal(markers, map[string]int{"live": 1, "marked": 1}) {
return fmt.Errorf("expected one delete marker for live and marked, instead got %v", markers)
}
if len(res.Versions) != len(versions) {
return fmt.Errorf("expected %v versions, instead got %v", len(versions), len(res.Versions))
}
return nil
}, withVersioning(types.BucketVersioningStatusEnabled))
}
func Versioning_Multipart_Upload_success(s *S3Conf) error {
testName := "Versioning_Multipart_Upload_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -7,4 +7,4 @@ body:
data:
Error:
Code: NotImplemented
Message: "A header you provided implies functionality that is not implemented."
Message: "A header you provided implies functionality that is not implemented"