diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 7e26840f0..79ea533b5 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -104,7 +104,7 @@ jobs: - name: Run multipart listing, cancellation and cleanup tests under race detector env: MINIO_API_REQUESTS_MAX: "10000" - run: go test -race ./cmd -run '^Test(Multipart|PaginateMultipartUploads|ListMultipartUploads|CleanupStaleUploads|ExtractModTime)' -count=1 -timeout=5m + run: go test -race ./cmd -run '^Test(ObjectCompleteMultipartUpload|Multipart|PaginateMultipartUploads|ListMultipartUploads|CleanupStaleUploads|ExtractModTime)' -count=1 -timeout=5m - name: Run listing metadata recovery, pagination and cancellation tests under race detector run: go test -race ./cmd -run '^Test(ListFallback|ListingCache|ListingMerge|ListingRecovery|ListingPending|ListingEntry|ListingSecondLock|StreamMetadataParts|LocalLockerListing|DistributedListing|MergeXLV2LatestNullCrossParity|CountXLV2MixedECGroups|ListCrossParity|ListObjectVersionsCrossParityNull|MigrationCrossParity)' -count=1 -timeout=5m diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index 0fac42c79..c17c45518 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -2419,13 +2419,17 @@ func (z *erasureServerPools) CompleteMultipartUpload(ctx context.Context, bucket // Hold write locks to verify uploaded parts, also disallows any // parallel PutObjectPart() requests. - uploadIDLock := z.NewNSLock(bucket, pathJoin(object, uploadID)) - wlkctx, err := uploadIDLock.GetLock(ctx, globalOperationTimeout) - if err != nil { - return objInfo, err + // An empty upload ID aliases the object lock already held above. + // Let the storage layer return its normal missing bucket/upload error. + if uploadID != "" { + uploadIDLock := z.NewNSLock(bucket, pathJoin(object, uploadID)) + wlkctx, err := uploadIDLock.GetLock(ctx, globalOperationTimeout) + if err != nil { + return objInfo, err + } + ctx = wlkctx.Context() + defer uploadIDLock.Unlock(wlkctx) } - ctx = wlkctx.Context() - defer uploadIDLock.Unlock(wlkctx) if z.SinglePool() { return z.serverPools[0].CompleteMultipartUpload(ctx, bucket, object, uploadID, uploadedParts, opts) diff --git a/cmd/restore-native_test.go b/cmd/restore-native_test.go index 577607ced..706dbc8a4 100644 --- a/cmd/restore-native_test.go +++ b/cmd/restore-native_test.go @@ -707,3 +707,35 @@ func (r *nativeRestorePartGateReader) Read(p []byte) (int, error) { } return r.ReadCloser.Read(p) } + +// Invalid upload paths must retain the backend's normal error precedence, +// rather than reacquiring the object lock as the upload lock. +func TestMultipartCompletionLockAlias(t *testing.T) { + for _, pools := range []int{1, 2} { + for _, id := range []string{"", ".", "./"} { + t.Run(fmt.Sprintf("pools=%d/id=%q", pools, id), func(t *testing.T) { + r := newRestoreSafetyTest(t, pools, versioning.Enabled) + for _, missingBucket := range []bool{false, true} { + bucket := r.bucket + if missingBucket { + bucket = "missing-bucket" + } + ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond) + _, err := r.z.CompleteMultipartUpload(ctx, bucket, r.object, id, nil, ObjectOptions{}) + cancel() + if id != "" { + if !errors.As(err, &MalformedUploadID{}) { + t.Errorf("malformed upload: %v", err) + } + } else if missingBucket { + if !isErrBucketNotFound(err) { + t.Errorf("missing bucket: %v", err) + } + } else if !errors.As(err, &InvalidUploadID{}) { + t.Errorf("invalid upload: %v", err) + } + } + }) + } + } +}