Fix 500s from malformed manifest digests and out-of-order final chunk PUTs - #4945
Merged
Conversation
Member
|
DCO isnt signed 😬 |
… PUTs `registry/handlers/blobupload.go`: `PutBlobUploadComplete` never validated `Content-Range` contiguity on the final chunked-upload PUT, so an out-of-order last chunk fell through to digest validation and returned 400 instead of the spec-mandated 416. d7a2b14 ("add content range handling in patch blob") added this contiguity check to `PatchBlobData` but never extended it to the PUT completion path, so this reuses the same check there. `registry/handlers/manifests.go`: `manifestDispatcher` silently treated any reference containing `:` that failed `digest.Parse` as a tag name, so a malformed digest reference (such as `sha256:baddigeststring`) reached the tag-based storage path and produced a 500 from an invalid filesystem path instead of a clean 400. Reject it as `ErrorCodeDigestInvalid` before the tag fallback instead. Verified against the OCI distribution-spec conformance suite -- the three previously-failing tests (chunked out-of-order + put chunk, invalid-digest-format manifest-put, invalid-digest-format manifest-get) now pass; 779 pass / 6 skip / 4 disabled / 0 fail out of 789 total. Assisted-By: "claude my eyes right out" Co-Authored-By: Tianon Gravi <admwiggin@gmail.com> Signed-off-by: Tianon Gravi <tianon.gravi@docker.com>
Contributor
Author
|
Real amateur hours at Tianon's house today 😭 Fixed! |
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
4 participants
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.
registry/handlers/blobupload.go:PutBlobUploadCompletenever validatedContent-Rangecontiguity on the final chunked-upload PUT, so an out-of-order last chunk fell through to digest validation and returned 400 instead of the spec-mandated 416. d7a2b14 ("add content range handling in patch blob") added this contiguity check toPatchBlobDatabut never extended it to the PUT completion path, so this reuses the same check there.registry/handlers/manifests.go:manifestDispatchersilently treated any reference containing:that faileddigest.Parseas a tag name, so a malformed digest reference (such assha256:baddigeststring) reached the tag-based storage path and produced a 500 from an invalid filesystem path instead of a clean 400. Reject it asErrorCodeDigestInvalidbefore the tag fallback instead.Verified against the OCI distribution-spec conformance suite -- the three previously-failing tests (chunked out-of-order + put chunk, invalid-digest-format manifest-put, invalid-digest-format manifest-get) now pass; 779 pass / 6 skip / 4 disabled / 0 fail out of 789 total.
Related work:
Assisted-By: "claude my eyes right out"