From 40bee4b7ba756567b16117c70a4c28311e220d6c Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sun, 6 Sep 2026 22:04:35 +0800 Subject: [PATCH] fix(delete): honor If-Match precondition on DeleteObject (#10) DeleteObject ignored the If-Match request header and always deleted the object (204). AWS S3 conditional deletes require that when If-Match is provided and does not match the object's current ETag, the delete is refused with 412 Precondition Failed and the object is left intact. The precondition is evaluated in erasureServerPools.DeleteObject, while the server-pool delete lock is held, before the delete-marker short-circuit and before any version is removed. It runs against the version that will actually be deleted: pinfo.ObjInfo for a normal delete, or the specifically addressed version (read under the held lock) for a version-scoped delete, since getPoolInfoExistingWithOpts strips VersionID. The check is a pure function (no ResponseWriter writes) and returns PreConditionFailed, which toAPIError maps to 412; CheckPrecondFn is cleared before lower layers run so the precondition is evaluated exactly once. Semantics: - If-Match mismatch on a live object -> 412, object preserved. - If-Match "*" requires a live object; a delete-marker-latest -> 412, and an explicitly addressed delete-marker version -> 412 (getObjectInfo returns the marker with MethodNotAllowed; the marker is the precondition target, not a 405). - SSE-C/SSE-KMS: compared against the public ETag derived without the customer key, so a satisfiable condition is never falsely rejected. - Explicit versionId -> evaluated against that version; a missing addressed version -> NoSuchVersion whether or not the key exists; a missing object (no versionId) -> NoSuchKey; no If-Match -> unchanged (including the unconditional version-scoped delete's error behavior). Scope: atomicity is guaranteed for a single erasure set (the default deployment). Multi-pool conditional-delete atomicity (concurrent writers across pools, cross-pool version selection) is tracked as a follow-up. Tests: pure-helper unit test (delete marker, "*", SSE-C without key); object-layer tests (unversioned match/mismatch/missing, versioned delete-marker-latest and addressed delete-marker version, explicit-version selection, missing version on present and absent keys, read-quorum loss); handler tests (412/204/wildcard/404) across both backends, with red/green demonstrated per guard. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe Signed-off-by: Feng Ruohang --- cmd/erasure-object-conditional-delete_test.go | 295 ++++++++++++++++++ cmd/erasure-server-pool.go | 54 ++++ cmd/object-handlers-common.go | 22 ++ ...object-handlers-conditional-delete_test.go | 146 +++++++++ cmd/object-handlers-delete-precond_test.go | 64 ++++ cmd/object-handlers.go | 21 ++ 6 files changed, 602 insertions(+) create mode 100644 cmd/erasure-object-conditional-delete_test.go create mode 100644 cmd/object-handlers-conditional-delete_test.go create mode 100644 cmd/object-handlers-delete-precond_test.go diff --git a/cmd/erasure-object-conditional-delete_test.go b/cmd/erasure-object-conditional-delete_test.go new file mode 100644 index 000000000..e0e1857ef --- /dev/null +++ b/cmd/erasure-object-conditional-delete_test.go @@ -0,0 +1,295 @@ +// Copyright (c) 2015-2025 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "bytes" + "context" + "net/http" + "testing" +) + +// TestDeleteObjectConditional verifies that a conditional DeleteObject +// (If-Match, wired through opts.CheckPrecondFn) is evaluated atomically at the +// object layer: a non-matching ETag must fail with PreConditionFailed and leave +// the object intact, a matching ETag must delete it, and an If-Match against a +// missing object must return a not-found error rather than silently succeeding. +func TestDeleteObjectConditional(t *testing.T) { + ctx := context.Background() + + obj, fsDirs, err := prepareErasure16(ctx) + if err != nil { + t.Fatal(err) + } + defer obj.Shutdown(context.Background()) + defer removeRoots(fsDirs) + + bucket := "test-bucket" + object := "test-object" + + if err = obj.MakeBucket(ctx, bucket, MakeBucketOptions{}); err != nil { + t.Fatal(err) + } + + if _, err = obj.PutObject(ctx, bucket, object, + mustGetPutObjReader(t, bytes.NewReader([]byte("test-value")), + int64(len("test-value")), "", ""), ObjectOptions{}); err != nil { + t.Fatal(err) + } + + objInfo, err := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + existingETag := objInfo.ETag + + // If-Match with a wrong ETag must fail and preserve the object. + t.Run("wrong-etag-precondition-failed", func(t *testing.T) { + opts := ObjectOptions{ + HasIfMatch: true, + CheckPrecondFn: func(oi ObjectInfo) bool { + return !isETagEqual(oi.ETag, "wrong-etag") + }, + } + if _, err := obj.DeleteObject(ctx, bucket, object, opts); !isErrPreconditionFailed(err) { + t.Errorf("expected PreConditionFailed, got: %v", err) + } + if _, err := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{}); err != nil { + t.Errorf("object must still exist after a failed conditional delete, got: %v", err) + } + }) + + // If-Match against a missing object must return a not-found error. + t.Run("missing-object-not-found", func(t *testing.T) { + opts := ObjectOptions{ + HasIfMatch: true, + CheckPrecondFn: func(oi ObjectInfo) bool { + return !isETagEqual(oi.ETag, existingETag) + }, + } + _, err := obj.DeleteObject(ctx, bucket, "does-not-exist", opts) + if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) { + t.Errorf("expected ObjectNotFound/VersionNotFound, got: %v", err) + } + }) + + // If-Match with the correct ETag must delete the object (run last). + t.Run("correct-etag-succeeds", func(t *testing.T) { + opts := ObjectOptions{ + HasIfMatch: true, + CheckPrecondFn: func(oi ObjectInfo) bool { + return !isETagEqual(oi.ETag, existingETag) + }, + } + if _, err := obj.DeleteObject(ctx, bucket, object, opts); err != nil { + t.Errorf("expected a successful delete with matching ETag, got: %v", err) + } + if _, err := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{}); !isErrObjectNotFound(err) { + t.Errorf("object must be removed after a matching conditional delete, got: %v", err) + } + }) +} + +// TestDeleteObjectConditionalWithReadQuorumFailure verifies that a conditional +// (If-Match) DeleteObject does NOT proceed when the object's current state +// cannot be read due to read-quorum loss: without a verified ETag the delete +// must fail rather than remove the object blindly. +func TestDeleteObjectConditionalWithReadQuorumFailure(t *testing.T) { + ctx := context.Background() + + obj, fsDirs, err := prepareErasure16(ctx) + if err != nil { + t.Fatal(err) + } + defer obj.Shutdown(context.Background()) + defer removeRoots(fsDirs) + + z := obj.(*erasureServerPools) + xl := z.serverPools[0].sets[0] + + bucket := "test-bucket" + object := "test-object" + + if err = obj.MakeBucket(ctx, bucket, MakeBucketOptions{}); err != nil { + t.Fatal(err) + } + + if _, err = obj.PutObject(ctx, bucket, object, + mustGetPutObjReader(t, bytes.NewReader([]byte("test-value")), + int64(len("test-value")), "", ""), ObjectOptions{}); err != nil { + t.Fatal(err) + } + + objInfo, err := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{}) + if err != nil { + t.Fatal(err) + } + existingETag := objInfo.ETag + + // Simulate read-quorum loss by taking 8 of 16 disks offline (EC 8+8). + erasureDisks := xl.getDisks() + z.serverPools[0].erasureDisksMu.Lock() + xl.getDisks = func() []StorageAPI { + for i := range erasureDisks[:8] { + erasureDisks[i] = nil + } + return erasureDisks + } + z.serverPools[0].erasureDisksMu.Unlock() + + // Even with the correct ETag we must not delete: the current state (hence the + // ETag) cannot be verified under read-quorum loss. + opts := ObjectOptions{ + HasIfMatch: true, + CheckPrecondFn: func(oi ObjectInfo) bool { + return !isETagEqual(oi.ETag, existingETag) + }, + } + if _, err := obj.DeleteObject(ctx, bucket, object, opts); err == nil { + t.Error("expected an error for a conditional delete under read-quorum loss, got nil (object may have been deleted without ETag verification)") + } +} + +// TestDeleteObjectConditionalVersioned verifies conditional DeleteObject on a +// versioned bucket, where the precondition is evaluated at the server-pool layer +// against the version that will actually be removed: +// - If-Match "*" when the latest version is a delete marker must fail (412), +// because there is no live object to match. +// - An explicit versionId If-Match is evaluated against the addressed version, +// not the latest one (match deletes it, mismatch is refused). +// - An If-Match against a missing version returns VersionNotFound, which the +// handler maps to NoSuchVersion. +func TestDeleteObjectConditionalVersioned(t *testing.T) { + ctx := context.Background() + + obj, fsDirs, err := prepareErasure16(ctx) + if err != nil { + t.Fatal(err) + } + defer obj.Shutdown(context.Background()) + defer removeRoots(fsDirs) + + bucket := "test-bucket" + + if err = obj.MakeBucket(ctx, bucket, MakeBucketOptions{VersioningEnabled: true}); err != nil { + t.Fatal(err) + } + versioned := globalBucketVersioningSys.PrefixEnabled(bucket, "any") + if !versioned { + t.Fatalf("expected versioning to be enabled on %q", bucket) + } + + put := func(object, content string) ObjectInfo { + oi, perr := obj.PutObject(ctx, bucket, object, + mustGetPutObjReader(t, bytes.NewReader([]byte(content)), int64(len(content)), "", ""), + ObjectOptions{Versioned: versioned}) + if perr != nil { + t.Fatalf("put %q: %v", object, perr) + } + return oi + } + ifMatch := func(value string) CheckPreconditionFn { + return func(oi ObjectInfo) bool { + return deleteIfMatchPreconditionFailed(http.Header{}, value, oi) + } + } + + // If-Match "*" against a delete-marker-latest must fail with 412. + t.Run("wildcard-on-delete-marker-latest", func(t *testing.T) { + object := "dm-object" + put(object, "v1") + // Create a delete marker (unconditional), making the latest a delete marker. + if _, derr := obj.DeleteObject(ctx, bucket, object, ObjectOptions{Versioned: versioned}); derr != nil { + t.Fatalf("create delete marker: %v", derr) + } + opts := ObjectOptions{Versioned: versioned, HasIfMatch: true, CheckPrecondFn: ifMatch("*")} + if _, derr := obj.DeleteObject(ctx, bucket, object, opts); !isErrPreconditionFailed(derr) { + t.Errorf("expected PreConditionFailed for If-Match:* on a delete-marker-latest, got: %v", derr) + } + }) + + // Explicit versionId is evaluated against the addressed (older) version. + t.Run("explicit-version-selection", func(t *testing.T) { + object := "ver-object" + v1 := put(object, "first") + v2 := put(object, "second-longer") // v2 is now the latest with a different ETag + if v1.ETag == v2.ETag { + t.Fatalf("test setup: versions must have distinct ETags") + } + + // Mismatch: delete v2 with v1's ETag must be refused, v2 preserved. + mismatch := ObjectOptions{Versioned: versioned, VersionID: v2.VersionID, HasIfMatch: true, CheckPrecondFn: ifMatch(v1.ETag)} + if _, derr := obj.DeleteObject(ctx, bucket, object, mismatch); !isErrPreconditionFailed(derr) { + t.Errorf("expected PreConditionFailed deleting v2 with v1 ETag, got: %v", derr) + } + if _, gerr := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{VersionID: v2.VersionID}); gerr != nil { + t.Errorf("v2 must still exist after a refused conditional delete, got: %v", gerr) + } + + // Match: delete v1 with v1's ETag must succeed even though v1 is not latest. + match := ObjectOptions{Versioned: versioned, VersionID: v1.VersionID, HasIfMatch: true, CheckPrecondFn: ifMatch(v1.ETag)} + if _, derr := obj.DeleteObject(ctx, bucket, object, match); derr != nil { + t.Errorf("expected the addressed version to be deleted, got: %v", derr) + } + if _, gerr := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{VersionID: v1.VersionID}); !isErrVersionNotFound(gerr) { + t.Errorf("v1 must be gone after a matching conditional delete, got: %v", gerr) + } + if _, gerr := obj.GetObjectInfo(ctx, bucket, object, ObjectOptions{VersionID: v2.VersionID}); gerr != nil { + t.Errorf("v2 must remain after deleting v1, got: %v", gerr) + } + }) + + // If-Match against a missing version on an EXISTING key returns VersionNotFound. + t.Run("missing-version", func(t *testing.T) { + object := "missing-version-object" + put(object, "only") + opts := ObjectOptions{Versioned: versioned, VersionID: mustGetUUID(), HasIfMatch: true, CheckPrecondFn: ifMatch("anything")} + if _, derr := obj.DeleteObject(ctx, bucket, object, opts); !isErrVersionNotFound(derr) { + t.Errorf("expected VersionNotFound for If-Match on a missing version, got: %v", derr) + } + }) + + // If-Match against a missing version on an ABSENT key must also return + // VersionNotFound (NoSuchVersion), not NoSuchKey: the request addresses a + // specific version, which does not exist regardless of the key. + t.Run("missing-version-absent-key", func(t *testing.T) { + opts := ObjectOptions{Versioned: versioned, VersionID: mustGetUUID(), HasIfMatch: true, CheckPrecondFn: ifMatch("anything")} + if _, derr := obj.DeleteObject(ctx, bucket, "never-existed", opts); !isErrVersionNotFound(derr) { + t.Errorf("expected VersionNotFound for If-Match on a version of an absent key, got: %v", derr) + } + }) + + // If-Match "*" addressing a delete-marker VERSION by id must fail with 412, + // not 405: a delete marker has no entity-tag to match. getObjectInfo returns + // the marker alongside MethodNotAllowed; the precondition runs on the marker. + t.Run("wildcard-on-explicit-delete-marker-version", func(t *testing.T) { + object := "explicit-dm-object" + put(object, "live") + dm, derr := obj.DeleteObject(ctx, bucket, object, ObjectOptions{Versioned: versioned}) + if derr != nil { + t.Fatalf("create delete marker: %v", derr) + } + if !dm.DeleteMarker || dm.VersionID == "" { + t.Fatalf("expected a delete-marker version, got DeleteMarker=%v VersionID=%q", dm.DeleteMarker, dm.VersionID) + } + opts := ObjectOptions{Versioned: versioned, VersionID: dm.VersionID, HasIfMatch: true, CheckPrecondFn: ifMatch("*")} + if _, derr := obj.DeleteObject(ctx, bucket, object, opts); !isErrPreconditionFailed(derr) { + t.Errorf("expected PreConditionFailed for If-Match:* on an addressed delete-marker version, got: %v", derr) + } + }) +} diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index c0701c108..fa033bf1c 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -1167,6 +1167,14 @@ func (z *erasureServerPools) DeleteObject(ctx context.Context, bucket string, ob } // Acquire a write lock before deleting the object. + // + // NOTE: this lock is taken at the server-pool level. The conditional + // (If-Match) precondition below relies on this lock making the read-check- + // delete sequence atomic. That holds for a single erasure set: the write + // path (PutObject) locks at the destination set, which shares this lock's + // namespace only within one set. Multi-pool conditional-delete atomicity + // (concurrent writers across pools, cross-pool version selection) is a + // separate concern tracked as a follow-up. lk := z.NewNSLock(bucket, object) lkctx, err := lk.GetLock(ctx, globalDeleteOperationTimeout) if err != nil { @@ -1187,9 +1195,55 @@ func (z *erasureServerPools) DeleteObject(ctx context.Context, bucket string, ob if _, ok := err.(InsufficientReadQuorum); ok { return objInfo, InsufficientWriteQuorum{} } + // A conditional (If-Match) delete addressing a specific version treats an + // absent key as an absent version. getPoolInfoExistingWithOpts strips + // VersionID, so a missing key surfaces ObjectNotFound here even for a + // version-scoped delete; normalize it to VersionNotFound (NoSuchVersion), + // matching this function's tail. The unconditional path is unchanged. + if opts.CheckPrecondFn != nil && opts.VersionID != "" && isErrObjectNotFound(err) { + return objInfo, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID} + } return objInfo, err } + // Evaluate the conditional (If-Match) precondition while the write lock + // acquired above is held, before the delete-marker short-circuit and before + // any version is removed, so the object cannot change between the check and + // the delete. This is scoped to a single erasure set (see the note at the + // lock above): only there do the delete lock and the write path share the + // same lock namespace, making the check-then-delete atomic. + if opts.CheckPrecondFn != nil { + // pinfo.ObjInfo is the current latest version. getPoolInfoExistingWithOpts + // intentionally strips VersionID, so for a version-scoped delete read the + // specifically addressed version and evaluate the precondition against it. + checkInfo := pinfo.ObjInfo + if opts.VersionID != "" { + vopts := opts + vopts.NoLock = true // delete lock already held above + vopts.CheckPrecondFn = nil + vi, verr := z.serverPools[pinfo.Index].GetObjectInfo(ctx, bucket, object, vopts) + if verr != nil && (!isErrMethodNotAllowed(verr) || !vi.DeleteMarker) { + // Genuine read failure for the addressed version: a missing + // version -> VersionNotFound (NoSuchVersion), read-quorum loss, etc. + return objInfo, verr + } + // verr is nil for a live version, or MethodNotAllowed with a populated + // delete-marker ObjectInfo when the addressed version is a delete + // marker. In the latter case evaluate the precondition against the + // marker, which fails any If-Match (-> 412), rather than surfacing 405. + checkInfo = vi + } else if checkInfo.Name == "" { + // The current state could not be read (e.g. read-quorum loss); refuse + // the conditional delete rather than act on an unverified precondition. + return objInfo, InsufficientReadQuorum{} + } + if opts.CheckPrecondFn(checkInfo) { + return objInfo, PreConditionFailed{} + } + // Precondition satisfied; lower layers must not re-evaluate it. + opts.CheckPrecondFn = nil + } + // Delete marker already present we are not going to create new delete markers. if pinfo.ObjInfo.DeleteMarker && opts.VersionID == "" { pinfo.ObjInfo.Name = decodeDirObject(object) diff --git a/cmd/object-handlers-common.go b/cmd/object-handlers-common.go index abb4c49f8..90009112a 100644 --- a/cmd/object-handlers-common.go +++ b/cmd/object-handlers-common.go @@ -340,6 +340,28 @@ func canonicalizeETag(etag string) string { return etagRegex.ReplaceAllString(etag, "$1") } +// deleteIfMatchPreconditionFailed reports whether the If-Match precondition on a +// DeleteObject request fails, in which case the delete must be refused with 412. +// It is pure and never writes to the ResponseWriter: DeleteObject may evaluate +// the precondition off the request goroutine (e.g. during multi-pool cleanup). +// +// - A delete marker (a non-live latest version) has no entity-tag to match, so +// any If-Match value, including "*", fails against it. +// - "*" matches any existing live object, so it only requires existence. +// - A concrete ETag is compared against the object's public ETag. For +// SSE-C/SSE-KMS objects the public ETag is derived from the stored suffix +// without the customer key (getDecryptedETag), so a satisfiable condition is +// never rejected merely because the caller did not supply the key. +func deleteIfMatchPreconditionFailed(h http.Header, ifMatch string, oi ObjectInfo) bool { + if oi.DeleteMarker { + return true + } + if strings.TrimSpace(ifMatch) == "*" { + return false + } + return !isETagEqual(getDecryptedETag(h, oi, false), ifMatch) +} + // isETagEqual return true if the canonical representations of two ETag strings // are equal, false otherwise func isETagEqual(left, right string) bool { diff --git a/cmd/object-handlers-conditional-delete_test.go b/cmd/object-handlers-conditional-delete_test.go new file mode 100644 index 000000000..342a81791 --- /dev/null +++ b/cmd/object-handlers-conditional-delete_test.go @@ -0,0 +1,146 @@ +// Copyright (c) 2015-2025 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "bytes" + "context" + "encoding/xml" + "net/http" + "net/http/httptest" + "testing" + + "github.com/dustin/go-humanize" + "github.com/minio/minio/internal/auth" + xhttp "github.com/minio/minio/internal/http" +) + +// TestAPIDeleteObjectHandlerIfMatch verifies conditional DeleteObject behavior +// for the If-Match request header (AWS S3 conditional deletes): +// - a non-matching ETag must return 412 Precondition Failed and preserve the object, +// - a matching ETag (or "*") must delete the object and return 204, +// - a request without If-Match must be unaffected, +// - If-Match against a missing key must return 404 NoSuchKey (not the idempotent 204). +func TestAPIDeleteObjectHandlerIfMatch(t *testing.T) { + defer DetectTestLeak(t)() + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: testAPIDeleteObjectHandlerIfMatch, endpoints: []string{"DeleteObject"}}) +} + +func testAPIDeleteObjectHandlerIfMatch(obj ObjectLayer, instanceType, bucketName string, apiRouter http.Handler, + credentials auth.Credentials, t *testing.T, +) { + // putObject (re)creates an object and returns its ETag. + putObject := func(object string) string { + data := generateBytesData(1 * humanize.MiByte) + oi, err := obj.PutObject(context.Background(), bucketName, object, + mustGetPutObjReader(t, bytes.NewReader(data), int64(len(data)), "", ""), ObjectOptions{}) + if err != nil { + t.Fatalf("%s: failed to put object %q: %v", instanceType, object, err) + } + return oi.ETag + } + + // exists reports whether the object is still present. + exists := func(object string) bool { + _, err := obj.GetObjectInfo(context.Background(), bucketName, object, ObjectOptions{}) + return err == nil + } + + // doDelete issues a signed DELETE, optionally with an If-Match header. + doDelete := func(object, ifMatch string) *httptest.ResponseRecorder { + var hdrs map[string]string + if ifMatch != "" { + hdrs = map[string]string{xhttp.IfMatch: ifMatch} + } + req, err := newTestSignedRequestV4(http.MethodDelete, getDeleteObjectURL("", bucketName, object), + 0, nil, credentials.AccessKey, credentials.SecretKey, hdrs) + if err != nil { + t.Fatalf("%s: failed to create DELETE request: %v", instanceType, err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + return rec + } + + // (1) If-Match with a wrong ETag: 412 Precondition Failed, object preserved. + t.Run("wrong-etag-412", func(t *testing.T) { + object := "precond-wrong-etag" + putObject(object) + rec := doDelete(object, `"non-matching-etag"`) + if rec.Code != http.StatusPreconditionFailed { + t.Fatalf("%s: expected %d, got %d (body: %s)", instanceType, http.StatusPreconditionFailed, rec.Code, rec.Body.String()) + } + if !exists(object) { + t.Fatalf("%s: object must still exist after a failed conditional delete", instanceType) + } + }) + + // (2) If-Match with the correct ETag: 204 No Content, object removed. + t.Run("correct-etag-204", func(t *testing.T) { + object := "precond-correct-etag" + etag := putObject(object) + rec := doDelete(object, `"`+etag+`"`) + if rec.Code != http.StatusNoContent { + t.Fatalf("%s: expected %d, got %d (body: %s)", instanceType, http.StatusNoContent, rec.Code, rec.Body.String()) + } + if exists(object) { + t.Fatalf("%s: object must be removed after a matching conditional delete", instanceType) + } + }) + + // (3) If-Match "*" on an existing object matches any ETag: 204, object removed. + t.Run("wildcard-204", func(t *testing.T) { + object := "precond-wildcard" + putObject(object) + rec := doDelete(object, "*") + if rec.Code != http.StatusNoContent { + t.Fatalf("%s: expected %d, got %d (body: %s)", instanceType, http.StatusNoContent, rec.Code, rec.Body.String()) + } + if exists(object) { + t.Fatalf("%s: object must be removed after a wildcard conditional delete", instanceType) + } + }) + + // (4) No If-Match header: unchanged behavior, 204, object removed. + t.Run("no-ifmatch-204", func(t *testing.T) { + object := "precond-none" + putObject(object) + rec := doDelete(object, "") + if rec.Code != http.StatusNoContent { + t.Fatalf("%s: expected %d, got %d (body: %s)", instanceType, http.StatusNoContent, rec.Code, rec.Body.String()) + } + if exists(object) { + t.Fatalf("%s: object must be removed after an unconditional delete", instanceType) + } + }) + + // (5) If-Match against a non-existent key: 404 NoSuchKey, not the idempotent 204. + t.Run("missing-key-404", func(t *testing.T) { + rec := doDelete("precond-missing-key", `"some-etag"`) + if rec.Code != http.StatusNotFound { + t.Fatalf("%s: expected %d, got %d (body: %s)", instanceType, http.StatusNotFound, rec.Code, rec.Body.String()) + } + var apiErr APIErrorResponse + if err := xml.Unmarshal(rec.Body.Bytes(), &apiErr); err != nil { + t.Fatalf("%s: failed to parse error response: %v", instanceType, err) + } + if apiErr.Code != "NoSuchKey" { + t.Fatalf("%s: expected error code NoSuchKey, got %q", instanceType, apiErr.Code) + } + }) +} diff --git a/cmd/object-handlers-delete-precond_test.go b/cmd/object-handlers-delete-precond_test.go new file mode 100644 index 000000000..53c80a39c --- /dev/null +++ b/cmd/object-handlers-delete-precond_test.go @@ -0,0 +1,64 @@ +// Copyright (c) 2015-2025 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "net/http" + "testing" + + "github.com/minio/minio/internal/crypto" +) + +// TestDeleteIfMatchPreconditionFailed exercises the pure If-Match evaluation for +// DeleteObject, including delete markers, the "*" wildcard, and SSE-C objects +// whose public ETag must be derived without the customer key. +func TestDeleteIfMatchPreconditionFailed(t *testing.T) { + const plainETag = "d41d8cd98f00b204e9800998ecf8427e" // 32-char MD5, returned as-is + + // SSE-C object: the stored ETag is longer than 32 chars and its public form + // is the trailing 32 chars, derived by getDecryptedETag without any key. + ssecPublic := "11112222333344445555666677778888" + ssecStored := "abcdef0123456789abcdef0123456789" + ssecPublic // 64 chars + ssecMeta := map[string]string{crypto.MetaSealedKeySSEC: "test-sealed-key"} + + testCases := []struct { + name string + ifMatch string + oi ObjectInfo + want bool // true => precondition failed (delete refused, 412) + }{ + {"live matching etag", plainETag, ObjectInfo{ETag: plainETag}, false}, + {"live matching quoted etag", `"` + plainETag + `"`, ObjectInfo{ETag: plainETag}, false}, + {"live non-matching etag", "0badbeef0badbeef0badbeef0badbeef", ObjectInfo{ETag: plainETag}, true}, + {"live wildcard", "*", ObjectInfo{ETag: plainETag}, false}, + {"live wildcard padded", " * ", ObjectInfo{ETag: plainETag}, false}, + {"delete marker concrete etag", plainETag, ObjectInfo{DeleteMarker: true}, true}, + {"delete marker wildcard", "*", ObjectInfo{DeleteMarker: true}, true}, + {"ssec matching public etag", ssecPublic, ObjectInfo{ETag: ssecStored, UserDefined: ssecMeta}, false}, + {"ssec wildcard", "*", ObjectInfo{ETag: ssecStored, UserDefined: ssecMeta}, false}, + {"ssec non-matching etag", "99998888777766665555444433332222", ObjectInfo{ETag: ssecStored, UserDefined: ssecMeta}, true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if got := deleteIfMatchPreconditionFailed(http.Header{}, tc.ifMatch, tc.oi); got != tc.want { + t.Errorf("deleteIfMatchPreconditionFailed(%q) = %v, want %v", tc.ifMatch, got, tc.want) + } + }) + } +} diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index a7f046421..fcb834821 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -2947,6 +2947,20 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. return } + // If-Match conditional delete (see AWS S3 conditional deletes). Delete the + // object only if its current ETag matches the client-supplied value, + // otherwise the request is refused with 412 Precondition Failed and the + // object is left intact. The precondition is evaluated in + // erasureServerPools.DeleteObject, against the version that will actually be + // removed, while the delete lock is held, so the object cannot change + // between the ETag check and the delete. + if ifMatch := r.Header.Get(xhttp.IfMatch); ifMatch != "" { + opts.HasIfMatch = true + opts.CheckPrecondFn = func(oi ObjectInfo) bool { + return deleteIfMatchPreconditionFailed(r.Header, ifMatch, oi) + } + } + rcfg, _ := globalBucketObjectLockSys.Get(bucket) if rcfg.LockEnabled && opts.DeletePrefix { apiErr := toAPIError(ctx, errInvalidArgument) @@ -3012,6 +3026,13 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. return } if isErrObjectNotFound(err) || isErrVersionNotFound(err) { + if opts.HasIfMatch { + // A conditional (If-Match) delete cannot satisfy its + // precondition against a missing object, so surface the + // not-found error instead of the idempotent 204 response. + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) + return + } // Send an event when the object is not found objInfo.Name = object objInfo.VersionID = opts.VersionID