Fix #1399 - Add a grammar corresponding to the parsing algorithm

[recvfrom]

This PR add a grammar that corresponds to the algorithm parsing section.

A grammar currently exists in section 4.4.1 but it's much stricter than what user agents actually enforce, and there have been several instances where that grammar was assumed to correspond with the cookie parsing algorithm defined in section 5. By having a separate grammar in section 5, hopefully we can reduce confusion while also preserving the high-level view that a grammar provides.

I'm not sure whether it's worth keeping around the existing grammar in section 4.4.1, but in this initial work I have not attempted to remove it.

CC @sbingler and @miketaylr

[sbingler]

I'm not sure whether it's worth keeping around the existing grammar in section 4.4.1, but in this initial work I have not attempted to remove it.

4.4.1 defines the behavior for a well behaved server, while your addition defines what UAs must accept to be compatible with the real world (and misbehaving servers). I think 4.4.1 still serves a useful purpose and I think we should keep it.

[recvfrom]

I believe I've address all feedback with the most recent commit, but please let me know if I missed anything.

[reschke]

This is a pattern of ABNF definition we're not using in http-core anymore. Note that by defintion, any valid cookie-av will match extension-av (right?).

It would be better to just define the common syntax of any cookie-av, and then define seperate ABNFs for the individual values.

[recvfrom]

Good point - I moved the attribute grammars into separate ABNFs. Thanks for the feedback!

[reschke]

At the time of parsing when "Expires" is detected, the parsing into name and value has already happened.

Thus, I would just define the syntax for the value, as in:

expires-v = cookie-date

Note this is how multi-level grammars are defined in the core HTTP specs as well.

[recvfrom]

Ah, I see - thank you for the example @reschke.

It seems like we'd lose a bit of completeness if we only include the attribute value in the grammar. For instance, the grammar wouldn't explicitly tie the "Expires" keyword to the Expires attribute, and wouldn't convey whether "ExPiReS" is treated the same way as "Expires". Having the grammar format used here align with the convention in the core HTTP specs seems desirable as well, though.

Does anyone else have thoughts on this?

[reschke]

. For instance, the grammar wouldn't explicitly tie the "Expires" keyword to the Expires attribute

It by definition can't do that anyway, as long as the grammar allows for extensibilty - an ABNF-driven parser would simply switch to the code branch for new attributes, and then the value syntax would remain unconstrained.

You really can't do this in ABNF; it needs to be done in prose.

[recvfrom]

If our goal is to have an ABNF-driven parser function correctly, it seems like we'd need to go back to the using the previous definition of cookie-av. To address the original concern of "any valid cookie-av will match extension-av", maybe we could do something like the following:

cookie-av          = expires-av / max-age-av / domain-av /
                     path-av / secure-av / httponly-av /
                     samesite-av / extension-av

extension-av       = 1*cookie-name-octet BWS extension-eq-value BWS
extension-eq-value = "" / ("=" BWS optional-value)
    ; attributes solely matching extension-av should be treated as unrecognized by
    ; cookie parsers strictly conforming to this RFC

What do you think?

[reschke]

I don't think you can solve this by adding prose (comments) to the ABNF.

The ABNF should be something that can translate to how a parser processes the input. Parsers in general parse first generic structures, then look at specific micro syntaxes of values.

That's what we're doing in the base spec, and that's what I'll keep suggesting to do.