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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-09-06 22:04:35 +08:00
parent e7d0e62f16
commit 40bee4b7ba
6 changed files with 602 additions and 0 deletions
+54
View File
@@ -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)