Skip to content

fix(uploads): first publish wins ownership of a content-addressed key - #387

Merged
qu0b merged 1 commit into
masterfrom
qu0b/uploads-first-publish-wins
Oct 23, 2026
Merged

qu0b merged 1 commit into
masterfrom
qu0b/uploads-first-publish-wins

Conversation

@qu0b

@qu0b qu0b commented Oct 23, 2026

Copy link
Copy Markdown
Member

Why

Follow-up to #384, addressing redpandabot's re-review. Publish did StatObject then PutObject with nothing tying the two together. Two concurrent first publishes of identical bytes under the same name both saw NoSuchKey, and whichever PUT landed last became the recorded owner. Someone who knew the exact bytes and name could race the original publisher, become the owner, and then delete the URL the victim shares.

What

  • Conditional create: a first publish (object absent at stat) sends If-None-Match: *. R2 documents this as evaluated atomically against committed state on PutObject: of racing writers, at most one succeeds and the rest get 412 (S3 API compatibility).
  • Losing the race is success: the key is content-addressed (hash prefix + name), so the winner stored exactly these bytes. The handler returns the URL; ownership stays with the first writer.
  • Single-part only: DisableMultipart is set, because R2 documents no conditional support on Create/CompleteMultipartUpload. Uploads are capped at 100 MiB (max_object_bytes), far below R2's 5 GiB single-PUT limit. Previously, files over 16 MiB went multipart.
  • Re-publishing an existing object is unchanged: an unconditional overwrite that keeps the existing owner (or lack of one).

Testing

  • The fake S3 now honours If-None-Match: * with an S3-shaped 412. A beforePut hook stages the competing writer exactly between stat and put.
  • New TestUploadsHandlerConcurrentFirstPublishKeepsFirstOwner: the loser gets the URL but can't delete; the winner can. Mutation-checked: dropping the conditional header fails the test (the loser becomes owner), and treating 412 as an error fails it (502).
  • make lint clean; go test -race ./... passes.
  • Not yet exercised against live R2 (no S3 credentials outside the proxy). Worth one parallel double-publish on staging once this ships.

Address review on #384: publish did stat-then-put, so two concurrent first
publishes of identical bytes both saw NoSuchKey and the last PUT became the
recorded owner. A first publish now sends If-None-Match: * (evaluated
atomically by R2 on single-part PutObject), and multipart is disabled so the
condition always applies. Losing the race returns the URL: the winner stored
the same bytes under the same name.
@qu0b
qu0b merged commit ea082fa into master Oct 23, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant