mirror of
https://github.com/pgsty/minio.git
synced 2026-11-10 17:45:32 +03:00
fix(pools): check copy names before locking them
CopyObject locks its source and destination before it reads the source, and lockObjects merged two requests only when their raw names were equal. A lock is taken on the cleaned name, so an invalid source such as a//x or a/./x and the destination a/x asked for a read lock and a write lock on one resource: the copy waited on itself until the operation timeout and answered 503, holding the key's lock in short windows meanwhile. Before the lock-domain change the invalid source was refused at once, when the source was read. This reached single nodes, multi-pool deployments and single pools whose sets share hosts. - Check the source with checkGetObjArgs before any lock, the first check and the first error the release reported. Lock the destination only when its name is valid. An invalid destination fails at the copy's own destination check before it writes, after a missing source (404) or a failed source precondition (412), as before. - lockObjects merges requests by the resource the lockers see, the cleaned bucket/object, within one lock table: the object's set where the sets of a pool have their own lockers, else one table. This keeps aliases on a shared local table from conflicting and never merges requests of different distributed domains. Tests: aliased sources and destinations are refused with 400 at once in every lock layout and two pools; an invalid destination keeps the 404 and 412 precedence; lockObjects merges an alias with its clean name. The alias and merge tests fail without the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
+12
-1
@@ -1483,8 +1483,19 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
|
||||
// request's context. Both locks are released once the copy has been
|
||||
// written: a synchronous replication below reads the destination with its
|
||||
// own lock.
|
||||
//
|
||||
// Names are checked before they are locked: a lock is taken on the
|
||||
// cleaned name, so an invalid alias such as a//x would lock the resource
|
||||
// of a/x a second time. The source is checked first, as reading it did
|
||||
// before. An invalid destination is not locked; the copy fails at its
|
||||
// own destination check before it writes, after the source errors that
|
||||
// take precedence over it.
|
||||
if err := checkGetObjArgs(ctx, srcBucket, srcObject); err != nil {
|
||||
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
|
||||
return
|
||||
}
|
||||
locks := []objectLockRequest{{bucket: srcBucket, object: srcObject}}
|
||||
if !remoteCallRequired {
|
||||
if !remoteCallRequired && checkCopyObjArgs(ctx, dstBucket, dstObject) == nil {
|
||||
locks = append(locks, objectLockRequest{bucket: dstBucket, object: dstObject, write: true})
|
||||
}
|
||||
lkctx, unlock, err := objectAPI.lockObjects(ctx, locks...)
|
||||
|
||||
Reference in New Issue
Block a user