Re: Working Group Last Call: Structured Headers for HTTP

On 29.01.2020 18:11, Tommy Pauly wrote:
> ...

Below is my (slightly late feedback).

I was going to ask that the spec to be aligned with the recent
terminology clarification we did in the core specs (and which are not
yet published).

In fact, this is already addressed in the editor's draft, see current
diffs at
<https://greenbytes.de/tech/webdav/draft-ietf-httpbis-header-structure-latest-from-previous.diff.html>.
In encourage all participants to review these changes...

That said, the title probably should also change from "Structured
Headers for HTTP" to something like "Structured Field Values for HTTP
Headers and Trailers".

I did not really review the algorithms; this essentially requires
writing code. I'm not really happy with the statement that in case of
conflicts, the algorithms have higher precedence than the ABNF.



Other than that; I found one major issue:

In 3.3.4:

   sh-token = ( ALPHA / "\*" ) *( tchar / ":" / "/" )

This should be

   sh-token = ( ALPHA / "*" ) *( tchar / ":" / "/" )

...right?

Minor issues:

In Section 1:

At the end of the section (top level), there should be a paragraph break
between the two sentences (so that each Section 2, 3, and 4 are treated
consistently)

In Section 1.1:

"In doing so, uses the VCHAR, SP, DIGIT, ALPHA and DQUOTE rules from
[RFC5234]. It also includes the tchar rule from [RFC7230]."

...should say..: "In doing so, *it* uses..."

In Section 2:

"Specify the type of the header field itself; either Dictionary (Section
3.2), List (Section 3.1), or Item (Section 3.3)."

Maybe sort by section number (swap Dictionary and List)?

Further down below it talks about the letter "Q" and the letter 'q'.
Please make the quoting consistent.

In the example spec text:

> "fooUrl" contains a URI-reference (Section 4.1 of
> [RFC3986], Section 4.1). If its value is not a valid URI-reference,
> that URL MUST be ignored. If its value is a relative reference
> (Section 4.2 of [RFC3986]), it MUST be resolved (Section 5 of
> [RFC3986]) before being used.

1. remove ", Section 4.1"

2. use of "URL" comes as as surprise here, maybe just say "field value"


In Section 3.1.1:

"In HTTP headers, inner lists..." - I was going to say that this seems
to be unnecessary verbose (here and in other places), but this has
already been fixed in the latest editor's draft.

In Section 3.1.2:

ABNF:

parameter     = ";" *SP param-name [ "=" param-value ]

Prose:

"...parameters are separated from their item or inner-list and each
other by semicolons."

So there's an unfortunate mismatch between the ABNF production
"parameter" (which already includes the ";") and the use of "parameter"
in the prose. Maybe worth addressing in the ABNF.


In Section 3.2:

> Members whose value is Boolean true MUST omit that value when serialised, unless it has parameters. For example, here both “b” and “c” are true, but “c”’s value is serialised because it has parameters:
>
> Example-DictHeader: a=?0, b, c=?1; foo=bar

I guess this is needed in order to avoid ambiguities, but this outcome
is really a bit strange. Is there a summary of how we got to that
special case somewhere (github issue?)


In Section 3.3:

> An item is can be a integer (Section 3.3.1), decimal (Section 3.3.2), string (Section 3.3.3), token (Section 3.3.4), byte sequence (Section 3.3.5), or Boolean (Section 3.3.6).

Please be consistent in upper/lowercasing...

In Section 3.3.2:

> Decimals are numbers with an integer and a fractional component. The Integer component has at most 12 digits; the fractional component has at most three digits.

Same here.

In Section 4.1:

Item 2 and 3 should be swapped (to reflect the order of the definitions
in the spec).

Item 5 seems to be unreachable by definition.

In Section 4.1.3:

Section references in the list should be wrapped in "(" and ")"

In Section 5:

s/draft/specification/ or "document"

Appendix A:

Acknowledgements should be in a unnumbered appendix at the end.

That said, is mentioning exactly one WG member the right thing here?

Appendix C:

> A generic implementation of this specification should expose the top-level parse (Section 4.2) and serialize (Section 4.1) functions

Maybe swap the two parts (Sections mentioned in reverse order really
trip me up)

Best regards, Julian

Received on Saturday, 22 February 2020 18:07:07 UTC