httpwg / http-extensions

HTTP Extensions in progress
https://httpwg.org/http-extensions/
445 stars 146 forks source link

Clarify digest-algorithm syntax with parameters (or drop parameters). #850

Closed ioggstream closed 2 years ago

ioggstream commented 5 years ago

I expect

This spec either:

RFC3230 stated that:

   For some algorithms, one or more parameters may be
   supplied.

      digest-algorithm = token

   The BNF for "parameter" is as is used in RFC 2616 [4].  All digest-
   algorithm values are case-insensitive.

but:

In this draft we preserved that sentence but:

In #1213 there's a possible fix to support parameters, like the following

Digest: myhash=dcRDgR2GM35DluAV13PzgnG6+pvQwPywfFvAu1UeFrs=; param1=40

Notes

Use cases are parametrizable digest-algorithms like mi-sha256.

Another solution could be to deprecate field parameters in digest: parameters could still be adopted serializing them in the digest-value, and leaving the specification/serialization to the definition of the digest-algorithm. eg:

Digest: myhash=40/dcRDgR2GM35DluAV13PzgnG6+pvQwPywfFvAu1UeFrs
ioggstream commented 4 years ago

@LPardue @reschke I don't know of any parameter in the wild, but removing that will deviate from RFC 3230.

If we want to remove parameters, we should explicit:

bikeshed=bs:8|offset:2|8E9q0okq

reschke commented 4 years ago

FWIW, deviating would fine in this case. One point in revising specs is to remove unused stuff.

ioggstream commented 4 years ago

@reschke I agree. My question is whether it's better to:

Guidance on that is welcome.

LPardue commented 4 years ago

@mnot removal of Digest parameters would be a case where our update work deviates a bit further from what we might have initially had in mind. As Julian points out its fine to do so, however I want to make this more visibile before we take a decision.

ioggstream commented 4 years ago

I want to make this more visibile before we take a decision

@LPardue I tweeted on that https://twitter.com/ioggstream/status/1295292991335268352 :)

To me leaving the parameter definition to the algorithm specification is fine, eg https://github.com/httpwg/http-extensions/issues/850#issuecomment-653931895

LPardue commented 4 years ago

I'll send an email to the HTTP WG list too.

reschke commented 4 years ago

+1 for removing parameters.

bsdphk commented 4 years ago

+0 on removing parameters, but I want to strongly suggest that any examples of encoded parameters are expressed in terms of Structured Fields, even if the entire header is not compliant.

ie:

bikeshed=:SD8E9q0okq:

where the parameters are encoded in the first two base64 encoded bytes.

LPardue commented 4 years ago

Mailing list thread: https://lists.w3.org/Archives/Public/ietf-http-wg/2020JulSep/0092.html

ioggstream commented 4 years ago

@reschke @mnot brief recap after discussion.

We propose to deprecate parameters because:

The proposal:

OT: I agree that making digest a SF would have resolved the issue with just some advice of not using parameters as input values when defining new digest-algorithms

ioggstream commented 4 years ago

@LPardue @reschke As parameters are now obsoleted, we have to decide whether to provide instructions for the transition. The actual draft just deprecated parameters and does not allow their presence. If you think the current behavior is fine, imho we could merge #1259 and cutoff -04

Current behavior

The presence of a parameter in any representation-data-digest will invalidate the syntax of the whole Digest value.

Hyp 1

add the parameter definition to the ABNF and explicit that they SHOULD|MUST be ignored, while the checksum should be processed

Hyp 2

add the parameter definition to the ABNF and explicit that if a representation-data-digest contains parameter only this checksum should be ignored

reschke commented 4 years ago

If we have no evidence of parameters in use, why would we need to consider a transition? Don't make things more complicated than they need to be.

ioggstream commented 2 years ago

@LPardue @martinthomson

We deprecated parameters after this ml thread but the discussion arose again after the migration to SF.

SF allows dropping the mention of them without formally deprecating, but I see some space for non interoperable behaviors. Parameters could apply both to specific algorithms and to all algorithms, eg.

Content-Digest: sha-256=:fafafa==:; q=1

Moreover we haven't defined any parameter yet.

Some questions we might be able to answer:

LPardue commented 2 years ago

My thoughts are:

SF allows dropping the mention of them without formally deprecating, but I see some space for non interoperable behaviors. Parameters could apply both to specific algorithms and to all algorithms, eg.

Content-Digest: sha-256=:fafafa==:; q=1

By default, receivers would ignore q=1 in your example. If the sender is expecting something to different to happen in the protocol due presence of q=1, for interoperability they have to do the work to write spec that tells the receiver what to do.

In terms of paramaterizing algorithms, based on the evidence so far, "you ain't gonna need it". And if that turns out to be wrong, then we can spend to time to debate the merits of structured fields parameters vs in-band encoding inside the Bytes. But we don't need to speculate now IMO.

ioggstream commented 2 years ago

implementations ignore unknown parameters as already instructed to by structured fields

Ok to defer the issue to future specs using parameters: I suppose the specs don't need further changes then.

LPardue commented 2 years ago

I'll make a PR that will capture how parameters are treated today and call out extensibility.

ioggstream commented 2 years ago

@LPardue are you ok to close this issue without further changes to the document, according to your comments in https://github.com/httpwg/http-extensions/issues/850#issuecomment-1055642386 ?

LPardue commented 2 years ago

I'm struggling to come up with any text to add that won't overcomplicate things. Let's close for now and see if this comes up again during last call.