Minor points in draft 12

Hi,

I finally got round to reading draft 12 end-to-end. Overall, it's an impressive piece of work. Congrats to all involved. However, I noticed a few minor places where there are potential ambiguities, places where the language could be a bit clearer and bits that confuse me:

"Pad High:  Padding size high bits.  This field is only present if the PAD_HIGH flag is set."
"Pad Low:  Padding size low bits.  This field is only present if the PAD_LOW flag is set."
"E: A single bit flag indicates that the stream dependency is exclusive, see Section 5.3.  This field is optional and is only present if the PRIORITY flag is set."
It's a bit ambiguous just from the text if the fields are there but set to all zero or missing completely from the byte stream? If the bytes were missing it would make no sense for a single bit to be so.

"The client makes an HTTP/1.1 request that includes an Upgrade header field identifying HTTP/2 with the "h2c" token."
What should happen if the client sends an "h2" upgrade? Ignore or error?

"The maximum concurrent streams setting is specific to each endpoint and applies only to the peer that receives the setting."
I'm assuming this is a hop-by-hop setting which an intermediary can change but it's not 100% clear from the text. It makes sense to me to have them independent - e.g. If the browser can support more streams than the server but the proxy can serve more streams by serving content from a cache. Alternatively a proxy could support a higher degree of concurrency to the server by combining requests from multiple clients if those clients are requesting static, PII-free resources from a server (e.g. a CDN).

"Endpoints MUST NOT exceed the limit set by their peer."
Presumably it would be an error if they did? I'm guessing PROTOCOL_ERROR?

"END_SEGMENT (0x2):"
As discussed elsewhere on the list, the term segment is used before it is defined. Since the doc later talks about "TCP segments" this becomes doubly confusing.

"The maximum size of a frame payload varies by frame type."
It would be good if these were laid out explicitly at some point, rather than simply implied by reading the protocol docs. It might encourage folks to write better validation code earlier.

"DATA frames MAY also contain arbitrary padding."
Padding with what? Zeroes or random noise? Before compression or after? The former answers would be simpler, the latter probably more secure.

"SETTINGS_HEADER_TABLE_SIZE ... The initial value is 4,096 bytes."
Shouldn't min and max values be defined for this as well?

"Upon receiving a SETTINGS frame with the ACK flag set, the sender of the altered parameters can rely upon their application."
I'm tempted to think a line mentioning what to do in the interim might be helpful - i.e. to assume the previous values. I can imagine some very subtle and evil bugs introduced if this is not done (e.g. for header compression table sizes).

"R: A single reserved bit." (under PUSH_PROMISE)
"The payload of a WINDOW_UPDATE frame is one reserved bit"
Might be worth adding the usual "always set to zero, always ignore" blurb for completeness.

"A receiver MUST treat the receipt of any other type of frame or a frame on a different stream as a connection error"
Maybe I'm missing something, but why the restriction on a frame on a different stream? I don't believe there's anywhere else in the protocol that such a restriction is made.

"Recipients of PUSH_PROMISE frames can choose to reject promised streams by returning a RST_STREAM referencing the promised stream identifier back to the sender of the PUSH_PROMISE."
With what error code? REFUSED_STREAM?

"The PUSH_PROMISE frame modifies the connection state as defined in Section 4.3. A PUSH_PROMISE frame modifies the connection state in two ways. "
Surely that first sentence is redundant?

"8.1.2. Examples... :method"
I think it would be better if the concept of these pseudo-headers (currently in 8.1.3) was introduced before they were used in examples.

"host=example.org"
I don't understand why this example uses a "host" header when the following one uses ":authority".

"The authority MUST NOT include the deprecated "userinfo" subcomponent for "http" or "https" schemed URIs."
So what do we do with the userinfo? In particular, I'm thinking of cases for HTTP/2 to HTTP/1.1 or FTP-over-HTTP.

"CONNECT ... Frame types other than DATA or stream management frames (RST_STREAM, WINDOW_UPDATE, and PRIORITY) MUST NOT be sent on a connected stream, and MUST be treated as a stream error (Section 5.4.2) if received."
What happens if a client receives a HEADER frame? Does it need to be minimally processed (for maintaining compression state)?

Plus there are a few typos I noticed:

"Until the remote peer receives and processes the frame bearing the END_STREAM flag, it might send frame of any of these types"
frames not frame

"Long-lived connections can result in endpoint exhausting the available range of stream identifiers."
Should be "an endpoint" or "the endpoint"

"GZIP compression ... GZip compression"
Case inconsistency

' "half-closed (local)", or "half closed (remote)" '
To hyphenate or not to hyphenate, that is the question. (this is more irritating than the others in that it makes it harder to search for - in chrome at least)

Regards,

Richard

Received on Saturday, 3 May 2014 06:47:45 UTC