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

Repository.Referrers does not properly detect the absence of the referrers API for registry.k8s.io #633

Closed
ktarplee opened this issue Nov 5, 2023 · 3 comments · Fixed by #635
Assignees
Labels
bug Something isn't working
Milestone

Comments

@ktarplee
Copy link
Contributor

ktarplee commented Nov 5, 2023

To reproduce

$ oras discover registry.k8s.io/sig-storage/csi-resizer:v1.8.0
Error: GET "https://accounts.google.com/v3/signin/identifier REDACTED": failed to decode response: invalid character '<' looking for beginning of value

The issue does not seem to be with in the oras CLI tool but rather the detection logic within the Repository's Referrers method to determine when a registry supports the referrers API or not. In the above example, registry.k8s.io was erroneously determined to implement the referrers API. In reality the response was a 307 that was followed to a login page for a user that was HTML and page returned a 200 so oras-go thinks that it supports the referrers API. Then when the referrers API is used, it is obvious that the JSON parser is trying to parse HTML. Not good.

So we need better detection logic for the referrers API. Having the request (made in pingReferrers) respond with a 200 is necessary but not sufficient in most cases. We could check that the content type in the response is actually an OCI index. It looks like there are lots of places withing oras-go where the Content-Type of the response is not checked to be a set of acceptable values before parsing the body. This is generally considered bad practice.

@shizhMSFT
Copy link
Contributor

shizhMSFT commented Nov 7, 2023

We could check that the content type in the response is actually an OCI index.

Good catch as the distribution-spec specifies

Upon success, the response MUST be a JSON body with an image index containing a list of descriptors. The Content-Type header MUST be set to application/vnd.oci.image.index.v1+json.

@shizhMSFT
Copy link
Contributor

@Wwwsylvia This Content-Type check is introduced by opencontainers/distribution-spec#452 after distribution-spec v1.1.0-rc3.

@Wwwsylvia
Copy link
Member

Thanks @ktarplee for catching this. Since you already have the PR, let's assign this to you.

@Wwwsylvia Wwwsylvia added this to the v2.4.0 milestone Nov 7, 2023
@Wwwsylvia Wwwsylvia added the bug Something isn't working label Nov 7, 2023
Wwwsylvia pushed a commit that referenced this issue Nov 21, 2023
When accessing the referrer's API, this adds a check to make sure the
content-type is an OCI index.

Also fixes the tests to properly set the content-type header in the
response.

Closes #633
Signed-off-by: Kyle M. Tarplee <kmtarplee@ieee.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

3 participants