fix: add required Content-Range header to PATCH test #576
Conversation
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
The spec requires Content-Range header for PATCH requests, but the conformance test "PATCH request with blob in body should yield 202 response" omits this requirement. This creates a mismatch between the spec and the test suite. The spec clearly states that for PATCH requests: - Content-Range is required - Must be inclusive on both ends - First chunk must begin with 0 - Must match regex ^[0-9]+-[0-9]+$ This fix adds the required Content-Range header to the test, ensuring it properly validates spec compliance.
coryodaniel
requested review from
dmcgowan,
jzelinskie,
jonjohnsonjr,
jdolitsky,
mikebrow,
stevvooe and
sudo-bmitch
as code owners
sudo-bmitch
requested changes
May 31, 2025
There was a problem hiding this comment.
This is the streaming upload, which we do not want to change to a chunked upload (that's already a separate test). See #506 for details. It's a deficiency in the spec, not in the conformance test.
|
Clarification in the spec is reasonable, removing from the conformance test is not the ideal solution though. |
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.
The spec requires Content-Range header for PATCH requests, but the conformance test "PATCH request with blob in body should yield 202 response" omits this requirement. This creates a mismatch between the spec and the test suite.
The spec clearly states that for PATCH requests:
This fix adds the required Content-Range header to the test, ensuring it properly validates spec compliance.
I implemented this a little hacky to illustrate, I wanted to make sure I wasn't misinterpreting the spec, but when I enforced the content-range header on PATCH in our registry, it caused the conformance test suite to fail.
Happy to move stuff into setup.go if you want it to be inline with the other setup variables. I didn't do it there since its not a "chunk" and that appears to be the naming convention. Anywho, thanks!