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

Fix: #968. Clarifications on Want-Digest. #1249

Merged
merged 9 commits into from Aug 24, 2020
Merged

Fix: #968. Clarifications on Want-Digest. #1249

merged 9 commits into from Aug 24, 2020

Conversation

ioggstream
Copy link
Contributor

This PR

clarifies that no normative parts were added to Want-Digest, only examples.

@ioggstream ioggstream changed the title Fix: #978. Clarifications on Want-Digest. Fix: #968. Clarifications on Want-Digest. Aug 18, 2020
@ioggstream ioggstream self-assigned this Aug 18, 2020
@LPardue
Copy link
Contributor

LPardue commented Aug 18, 2020

Hmm I think the issue might have conflated two things. 1) We are deprecating use of the MD5 algorithm in digests (including want-digest with md5 2) We are deprecating the use of content-MD5 as described in https://tools.ietf.org/html/rfc3230#section-5. I think we should expand the deprecate-contentMD5 section a little to explicitly state what want-digest with contentMD5 means in this draft i.e. is it ignored?. It's easy to confuse these things with the current text.

@ioggstream
Copy link
Contributor Author

I think we should expand the deprecate-contentMD5 section a little
to explicitly state what want-digest with contentMD5 means in this draft i.e. is it ignored?

To me the implementor is free to ignore or error on contentMD5: OTOH it's an unregistered algorithm.
We could just reserve that word to avoid that to be registered, but I don't like this kind of entries lingering around (eg. see httpwg/http-core#388 )

re-reading the draft, I found the message quite clear: forget md5 and all that relates to it.

@reschke
Copy link
Contributor

reschke commented Aug 18, 2020

We could just reserve that word to avoid that to be registered, but I don't like this kind of entries lingering around

Having the value in the registry makes a lot of sense, because that's what people are supposed to look up. It should just make clear that it's deprecated.

(eg. see httpwg/http-core#388 )

That's IMHO a bit misleading as "identity" is not a real content-coding.

@LPardue
Copy link
Contributor

LPardue commented Aug 18, 2020

My interpretation is different than Roberto's; RFC3230 registers the value contentMD5 for use in want-digest, and only want-digest.

   First, we add to the set of digest-algorithm values (in section
   4.1.1) the token "contentMD5", with the provision that this digest-
   algorithm MUST NOT be used in a Digest header field.

then

  The presence of the "contentMD5" digest-algorithm with a non-zero
   qvalue in a Want-Digest header field indicates that the sender wishes
   to receive a Content-MD5 header on messages associated with the
   Request-URI.

so a client sending want-digest: contentMD5;q=1 is asking for the server to provide a Content-MD5 in the response, independent of the Digest. That contentMD5 is missing from any registry seems like an oversight.

In the meantime, RFC 7231 has deprecated Content-MD5 and says:

   The Content-MD5 header field has been removed because it was
   inconsistently implemented with respect to partial responses.

So the right thing to do, I think, is to register this as an obsolete digest-algorithm. Whether that needs to be a SHOULD NOT be used in Want-Digest and MUST NOT be used in Digest, or a MUST NOT in both (probably unenforceable) I don't know.

@ioggstream
Copy link
Contributor Author

1- I'll register the algorithm in the table and mark it as Obsolete
2- PTAL https://github.com/httpwg/http-extensions/pull/1249/files#r472865789

@ioggstream
Copy link
Contributor Author

ioggstream commented Aug 24, 2020

@LPardue

So the right thing to do, I think, is to register this as an obsolete digest-algorithm. Whether that needs to be a SHOULD NOT be used in Want-Digest and MUST NOT be used in Digest, or a MUST NOT in both (probably unenforceable) I don't know.

@reschke

Having the value in the registry makes a lot of sense

Done :)

@ioggstream ioggstream merged commit 329e186 into master Aug 24, 2020
@ioggstream ioggstream deleted the ioggstream-978 branch August 24, 2020 14:04
richanna pushed a commit to richanna/http-extensions that referenced this pull request Oct 20, 2020
* Fix: httpwg#968. Clarifications on Want-Digest.
* Add contentMD5 to IANA table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants