oras-project / oras-go

ORAS Go library
https://oras.land
Apache License 2.0
170 stars 91 forks source link

Bug: oras-go is not actually getting `headerOCISubject` from response Header #699

Closed Two-Hearts closed 5 months ago

Two-Hearts commented 5 months ago

It might be more of an enhancement than a bug, but under function checkOCISubjectHeader, it says:

// If the "OCI-Subject" header is set, it indicates that the registry
// supports the Referrers API and has processed the subject of the manifest.

Although defined by distribution-spec, "OCI-Subject" is actually not a valid key for http.Response.Header. Keys in http.Response.Header are canonicalized. Therefore, given the below http response:

&http.Response{
    StatusCode: http.StatusCreated,
    Body:       io.NopCloser(bytes.NewReader([]byte(msg))),
    Header: http.Header{
        "OCI-Subject":  {"sha256:abcd..."},
    },
}

checkOCISubjectHeader will not run s.repo.SetReferrersCapability(true). And hence, not marking the registry as supporting the Referrers API. This is caused by oras-go directly getting the response header through resp.Header.Get("OCI-Subject"), which then searches for CanonicalMIMEHeaderKey of OCI-Subject, i.e. Oci-Subject, and this header is not in the above http.Response.

What's expected: As described by distribution-spec v1.1.0-rc4: https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc4/spec.md#pushing-manifests-with-subject, when response header OCI-Subject is present, s.repo.SetReferrersCapability(true) should be run.

Alternative: Update the comment stating that caller SHOULD use the http.response.Header.Set() method to set the OCI-Subject header.

shizhMSFT commented 5 months ago

It is a mock issue and the mock should use Header.Set() for a proper setup. In the real world, OCI-Subject header key returned by the server is transformed into Oci-Subject by the built-in golang http library.