Last Call Review of draft-ietf-httpbis-http2bis-06
review-ietf-httpbis-http2bis-06-tsvart-lc-ott-2021-11-28-00

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@ietf.org if you reply to or forward this review.

The Internet draft describes a revision of the HTTP/2 framing and optimized
representation, revising RFC 7540. The document is in a very good and essentially
ready to go, possibly subject to a few considerations and clarifications discussed
below.

Best,
Jörg

Section 5.1.1., 3rd para:

When a stream transitions out of
the "idle" state, all streams that might have been initiated by that
peer with a lower-valued stream identifier are implicitly
transitioned to "closed". 

Should one say "that might have been but were not initiated by that peers"
to make clear that this applies only to un-used streams?

Section 5.3.1
Maybe update the section heading to read
"Background on Priority in HTTP/2 as per RFC7540"?

Section 6.5:
The document implies that SETTINGS frames are ACKed in the order they are sent.
Should one make this explicit?

Last para of 6.5, top of page 37:
"If the sender of a SETTINGS frame does not receive an acknowledgement
within a reasonable amount of time, it MAY issue a connection error
(Section 5.4.1) of type SETTINGS_TIMEOUT."

Should one provide some intuition on what "reasonable" could mean? An example?
It seems, an HTTP/2 sender of a SETTINGS frame could know when this was passed
to the TCP layer and may have some insights on application layer RTTs.

Section 6.9.1: Concerning flow control, should one say something about avoiding
silly windows?

Section 8.1.1, p52 1st para: Probably just me, but I am mildly confused by the
statement about messages having a non-zero content-length field but still no
data in the data frames. The reference section 6.4 of the HTTP semantics draft
didn't resolve that. 

Section 8.2.1, p53: parse error?
"When a request message violates one of these requirements, an
implementation SHOULD generate a Section 15.5.1 of 400 (Bad Request)
status code [HTTP], [...]"

Section 8.4.2, 4th para:
"A client can use the SETTINGS_MAX_CONCURRENT_STREAMS setting to limit
the number of responses that can be concurrently pushed by a server.
Advertising a SETTINGS_MAX_CONCURRENT_STREAMS value of zero disables
server push by preventing the server from creating the necessary
streams.  This does not prohibit a server from sending PUSH_PROMISE
frames; clients need to reset any promised streams that are not
wanted. 

Confused: it disables server push, but the server can send PUSH_PROMISE frames
and then the client is supposed to reset a stream it wasn't willing to accept or
capable of accepting? Which stream number would the server legitimately put
into the PUSH_PROMISE?

There are a few cases where I am wondering if normative language should be used.
Examples include
-- 5.4.2, 2nd para, "sends an RST_STREAM frame"
-- 6.8, p43, 1st para, "the server can send another GOAWAY"
-- 6.8, p43, 2nd para, "the sender can discard frames"
-- 9.1, 3rd para, "clients can create"
There may be further occurrences of "can" or similar.