Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Oct 17, 2016

As Adrian Colley points out in #390, clients may not be aquiring the manifest over HTTP. In most cases, folks will know (from a Content-Type header, a descriptor media type, or other means) what type of blob they're dealing with before they look at the blob. I expect client behavior like:

If you can verify the digest, ignore Content-Type. If you can't verify the digest, respect the Content-Type and require it to match your expected media type (if any).

But in the absence of an HTTP-API spec I don't think we need to talk about this at all.

Fixes #390.

As Adrian Colley points out [1], clients may not be aquiring the
manifest over HTTP.  In most cases, folks will know (from a
Content-Type header, a descriptor media type, or other means) what
type of blob they're dealing with before they look at the blob.  I
expect client behavior like [2]:

  If you can verify the digest, ignore Content-Type.  If you can't
  verify the digest, respect the Content-Type and require it to match
  your expected media type (if any).

But in the absence of an HTTP-API spec I don't think we need to talk
about this at all.

[1]: opencontainers#390
[2]: opencontainers#390 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@jonboulle
Copy link
Contributor

jonboulle commented Oct 18, 2016

LGTM (though I'd also be happy to change this to a suggestion, e.g. "In the case of transferring images over HTTP clients COULD distinguish ..." or similar)

Approved with PullApprove

@wking
Copy link
Contributor Author

wking commented Oct 18, 2016

On Tue, Oct 18, 2016 at 07:46:35AM -0700, Jonathan Boulle wrote:

(though I'd also be happy to change this to a suggestion, e.g. "In
the case of transferring images over HTTP clients COULD distinguish
..." or similar)

I'm fine with that or the guidelines I quote in the topic post.
Either way, this sort of advice is not manifest-list specific. We can
push it into media-types.md or the considerations.md that is in flight
with #340. Would you like me to move it to media-types.md in this PR?

@philips
Copy link
Contributor

philips commented Oct 19, 2016

LGTM

I like @jonboulle's suggestion; lets do this in media-types.md

Approved with PullApprove

@wking
Copy link
Contributor Author

wking commented Oct 19, 2016

On Tue, Oct 18, 2016 at 09:07:21PM -0700, Brandon Philips wrote:

I like @jonboulle's suggestion…

“COULD distinguish” isn't much guidance ;). Do you have specific
concerns with the “If you can verify…” language I quote in 1? That
quote tries to set up the following behavior tree:

  1. You know the media type ahead of time.
    a. The Content-Type matches your expected media type.
    Use your expected media type to select the appropriate blob
    parser / validator.
    b. The Content-Type does not match the expected media type.
    i. You know the digest ahead of time.
    Use your expected media type to select the appropriate blob
    parser / validator. Possibly warn about the mismatch.
    ii. You don't know the digest ahead of time.
    Error out citing the mismatch.
  2. You don't know the media type ahead of time.
    Use Content-Type to select the appropriate blob parser / validator.

Would you change any of those? Recommend a different tree? Just
rephrase the wording for that tree?

@jonboulle
Copy link
Contributor

Do you have specific
concerns with the “If you can verify…” language I quote in [1]?

This seems fine. Please file a followup PR.

@jonboulle jonboulle merged commit de4926b into opencontainers:master Oct 19, 2016
wking added a commit to wking/image-spec that referenced this pull request Oct 19, 2016
Fill in more generic detail for the advice which was removed in
2c507b7 (manifest-list: Drop HTTP Content-Type sentence, 2016-10-17,
opencontainers#392).

Signed-off-by: W. Trevor King <[email protected]>
@wking wking deleted the no-http-headers branch October 20, 2016 04:19
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