Repository navigation
fix(uploads): first publish wins ownership of a content-addressed key - #387
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Follow-up to #384, addressing redpandabot's re-review. Publish did
StatObjectthenPutObjectwith nothing tying the two together. Two concurrent first publishes of identical bytes under the same name both sawNoSuchKey, 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
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).DisableMultipartis 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.Testing
If-None-Match: *with an S3-shaped 412. AbeforePuthook stages the competing writer exactly between stat and put.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 lintclean;go test -race ./...passes.