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

mediatypes: add band-aid for quay.io handling #82

Merged
merged 1 commit into from
Jan 12, 2019

Conversation

steveej
Copy link
Contributor

@steveej steveej commented Jan 12, 2019

Since recently quay.io appends ; charset=utf-8 in the content-type
header value for HEAD requests on the manifests endpoint. This patch
restores functionality as before the quay.io change.

@steveej steveej requested a review from lucab January 12, 2019 00:14
src/mediatypes.rs Outdated Show resolved Hide resolved
@steveej steveej force-pushed the fix-quay-content-type-parsing branch from 1e71fdf to 7b7df81 Compare January 12, 2019 00:20
@steveej steveej mentioned this pull request Jan 12, 2019
@steveej steveej changed the title mediatypes: add band-aid fix quay.io handling mediatypes: add band-aid for quay.io handling Jan 12, 2019
Since recently quay.io appends `; charset=utf-8` in the `content-type`
header value for HEAD requests on the `manifests` endpoint. This patch
restores functionality as before the quay.io change.
@steveej steveej force-pushed the fix-quay-content-type-parsing branch from 7b7df81 to 4f8a181 Compare January 12, 2019 00:23
@steveej steveej merged commit 13ed6e8 into camallo:master Jan 12, 2019
@steveej steveej deleted the fix-quay-content-type-parsing branch January 12, 2019 00:51
@lucab
Copy link
Member

lucab commented Jan 12, 2019

This PR is unfortunately wrong (and I fear the change on quay.io side too).

Media-types are part of the docker-registry API specifications. The V2S1 types are defined here and they don't have any additional (including charset) parameters.

Additionally, those manifests are explicitly compatible with application/json. JSON is a binary media type. As binary, it doesn't really have an outer charset, and the spec explicitly says that it doesn't have any additional (including charset) parameters.

@thomasmckay @josephschorr any chance to revert the change on quay, and align it back with IETF and docker-registry specs?

@josephschorr
Copy link

@lucab We can certainly revert, but we'd have to double check on what Docker is returning for manifests. The problem is the Docker documentation itself shows Content-Type: application/json; charset=utf-8 on some of the API returns, so it isn't clear whether that applies to the manifest type when it itself is application/json. I want to be sure before we go back and potentially break utf-8 encoded manifests.

@lucab
Copy link
Member

lucab commented Jan 14, 2019

For reference, the quay.io change was reverted in the meanwhile. It was breaking kubernetes CI too: kubernetes/kubernetes#72863 (comment)

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.

None yet

4 participants