Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed to use "bytes" unit for Content-Range to spec/test #203

Closed
wants to merge 1 commit into from

Conversation

Code-Hex
Copy link

According rfc7233, We need "bytes" prefix.

Grammar is here

 byte-content-range  = bytes-unit SP
                           ( byte-range-resp / unsatisfied-range )

@Code-Hex Code-Hex changed the title fixed to use "bytes" unit at Content-Range fixed to use "bytes" unit for Content-Range to spec/test Oct 28, 2020
@jdolitsky jdolitsky added this to the v1.0.0-rc2 milestone Oct 28, 2020
@pmengelbert
Copy link
Contributor

@Code-Hex I tested this fix against three different registries, and it caused all of them to fail the conformance tests. I appreciate the attention to detail in conforming to RFC 7233, but this spec is derived from the original Docker distribution specification, which itself was out of sync with that RFC.

Because of this, most registries do not expect the 'bytes' prefix, and adding it breaks their conformance. This would require most registries to code a solution, which is not ideal, especially so shortly before the 1.0 release.

@jdolitsky

@jdolitsky
Copy link
Member

+1. I'm not seeing any reference to RFC7233 in spec. Going to drop fro rc2 milestone

@jdolitsky jdolitsky removed this from the v1.0.0-rc2 milestone Nov 2, 2020
Base automatically changed from master to main March 9, 2021 17:38
@jdolitsky
Copy link
Member

although it would be better in theory to follow the RFC, it was discussed and it doesn't make sense to break people for this.

Please resubmit into spec.md with language vs. conformance code changes if you believe this should be optional behaviour

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.

3 participants