diff --git a/pkg/proxy/handlers/uploads.go b/pkg/proxy/handlers/uploads.go index f6f9a1ba..f10181cb 100644 --- a/pkg/proxy/handlers/uploads.go +++ b/pkg/proxy/handlers/uploads.go @@ -192,18 +192,25 @@ func (h *UploadsHandler) handlePut(w http.ResponseWriter, r *http.Request) { // Identical bytes map to the same key, so a re-publish keeps the existing // owner — or lack of one: ownerless legacy objects must not be adoptable. - uploader := uploaderFrom(r.Context()) + uploader, exists := uploaderFrom(r.Context()), false if info, err := h.client.StatObject(r.Context(), h.cfg.Bucket, key, minio.StatObjectOptions{}); err == nil { - uploader = info.UserMetadata[uploaderMetaKey] + uploader, exists = info.UserMetadata[uploaderMetaKey], true } else if minio.ToErrorResponse(err).Code != "NoSuchKey" { h.fail(w, http.StatusBadGateway, "checking object", err) return } - opts := minio.PutObjectOptions{ContentType: contentType} + // Single-part so R2 evaluates If-None-Match atomically (it does not for + // multipart); objects are capped well below R2's 5 GiB single-PUT limit. + opts := minio.PutObjectOptions{ContentType: contentType, DisableMultipart: true} if uploader != "" { opts.UserMetadata = map[string]string{uploaderMetaKey: uploader} } + // A first publish only creates: of two concurrent first publishes, the + // later one must not overwrite the earlier one's ownership. + if !exists { + opts.SetMatchETagExcept("*") + } // Types a browser would execute in the bucket origin (HTML, SVG, JS) are // forced to download — defuses phishing/XSS on a public bucket while charts, // PDFs and text render inline. The one exception is text/html when RenderHTML @@ -213,7 +220,10 @@ func (h *UploadsHandler) handlePut(w http.ResponseWriter, r *http.Request) { opts.ContentDisposition = fmt.Sprintf("attachment; filename=%q", name) } - if _, err := h.client.PutObject(r.Context(), h.cfg.Bucket, key, tmp, size, opts); err != nil { + // Losing the create race is success: the key is content-addressed, so the + // winner stored these exact bytes under this name. + if _, err := h.client.PutObject(r.Context(), h.cfg.Bucket, key, tmp, size, opts); err != nil && + (exists || minio.ToErrorResponse(err).StatusCode != http.StatusPreconditionFailed) { h.fail(w, http.StatusBadGateway, "putting object", err) return } diff --git a/pkg/proxy/handlers/uploads_test.go b/pkg/proxy/handlers/uploads_test.go index 63c64660..82038f7a 100644 --- a/pkg/proxy/handlers/uploads_test.go +++ b/pkg/proxy/handlers/uploads_test.go @@ -207,10 +207,12 @@ func TestDeletableKeyCoversBothPrefixes(t *testing.T) { } // fakeS3 is a minimal path-style S3 backend: enough of HEAD/PUT/DELETE for the -// uploads handler, keeping only each object's x-amz-meta-* headers. +// uploads handler, keeping only each object's x-amz-meta-* headers. beforePut, +// when set, runs once ahead of the next PUT to stage a concurrent writer. type fakeS3 struct { - mu sync.Mutex - objects map[string]http.Header + mu sync.Mutex + objects map[string]http.Header + beforePut func(objects map[string]http.Header, path string) } func (f *fakeS3) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -236,6 +238,15 @@ func (f *fakeS3) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("ETag", `"etag"`) case http.MethodPut: _, _ = io.Copy(io.Discard, r.Body) + if f.beforePut != nil { + f.beforePut(f.objects, r.URL.Path) + f.beforePut = nil + } + if _, exists := f.objects[r.URL.Path]; exists && r.Header.Get("If-None-Match") == "*" { + w.WriteHeader(http.StatusPreconditionFailed) + _, _ = io.WriteString(w, `PreconditionFailedAt least one of the pre-conditions you specified did not hold`) + return + } meta := http.Header{} for k, v := range r.Header { if strings.HasPrefix(k, "X-Amz-Meta-") { @@ -358,3 +369,21 @@ func TestUploadsHandlerRepublishDoesNotAdoptOwnerlessObject(t *testing.T) { t.Fatalf("delete after re-publishing ownerless object: status = %d, want 403", got) } } + +func TestUploadsHandlerConcurrentFirstPublishKeepsFirstOwner(t *testing.T) { + h, backend := fakeS3UploadsHandler(t) + + // alice's identical publish lands between mallory's stat and put. + backend.beforePut = func(objects map[string]http.Header, path string) { + objects[path] = http.Header{"X-Amz-Meta-Uploader": {"alice"}} + } + key := uploadAs(t, h, "mallory", "

shared

") + + if got := deleteAs(h, "mallory", key); got != http.StatusForbidden { + t.Fatalf("delete as race loser: status = %d, want 403", got) + } + + if got := deleteAs(h, "alice", key); got != http.StatusNoContent { + t.Fatalf("delete as race winner: status = %d, want 204", got) + } +}