- From: Simpson, Robby (GE Energy Management) <robby.simpson@ge.com>
- Date: Mon, 4 Aug 2014 19:34:33 +0000
- To: "ietf-http-wg@w3.org" <ietf-http-wg@w3.org>
Here are some miscellaneous comments based on my read of -14. Please let me know if there is some other way I should submit (bug tracker, etc.) Various ASCII art header figures show fields aligned on byte and word boundaries (e.g., "DATA Frame Payload", "HEADERS Frame Payload", "Setting Format", "PUSH_PROMISE Payload Format"). The text doesn't mention this at all. Is byte and word alignment intended? The plurality of the phrase "header block fragment" is ambiguous to me and seems to vary in the text. Can a HEADERS, PUSH_PROMISE, or CONTINUATION frame contain "fragments" or "a fragment"? In 6.6, "PADDED (0x8): Bit 4 being set indicates that the Pad Length field is present." This needs to also indicate that "Padding" may also be present, dependent on the Pad Length. Also "Padding: Padding octets" needs to state that it is only present if PADDING flag is set and Pad Length > 0. GOAWAY frame - does the "Additional Debug Data" count toward flow control? I think not, but I could see this field growing large. I understand we may not wish to block GOAWAY frames, but there may be a hole here with an unbounded size. (HEADERS, as a counter-example, at least has a limited size in SETTINGS). Is it OK just to rely on MAX_FRAME_SIZE? 9.2 states "Implementations of HTTP/2 MUST support TLS 1.2 [TLS12] for HTTP/2 over TLS." then "An implementation of HTTP/2 over TLS MUST use TLS 1.2 or higher with the restrictions on feature set and cipher suite described in this section." So which is it? == 1.2 or >= 1.2 In 6.5.2, SETTINGS_MAX_FRAME_SIZE initial value is defined "The initial value is 2^14 (16,384) octets." In 11.3 its 65536. Nits: End of page 9, beginning of page 10: Need a "For example:" or other introductory text. The text for the state "reserved (local)" and "reserved (closed)" could be made similar. "An endpoint MUST NOT send frames other than HEADERS or RST_STREAM in this state." and "A PRIORITY frame MAY be received in this state. Receiving any frames other than RST_STREAM, or PRIORITY MUST be treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR." vs. "An endpoint MAY send a PRIORITY frame in this state to reprioritize the reserved stream. An endpoint MUST NOT send any other type of frame other than RST_STREAM or PRIORITY." and "Receiving any other type of frame other than HEADERS or RST_STREAM MUST be treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR." The first and last paragraphs of 5.3 are a bit redundant: "A client can assign a priority for a new stream by including prioritization information in the HEADERS frame (Section 6.2) that opens the stream. For an existing stream, the PRIORITY frame (Section 6.3) can be used to change the priority." and "Prioritization information can be specified explicitly for streams as they are created using the HEADERS frame, or changed using the PRIORITY frame. Providing prioritization information is optional, so default values are used if no explicit indicator is provided (Section 5.3.5)." In 5.4.3, "If the TCP connection is torn down" should be something like "If the TCP connection is reset or closed". I'm not sure "torn down" is really defined. 6.1 and elsewhere: "The DATA frame defines the following flags:" This does not clearly indicate where the flags are set. We likely all know the field, but the draft does not state it. The phrase "This field is optional and is only present if the PRIORITY flag is set" occurs a few times. Suggest removing "is optional and". The phrase "A HEADERS frame that is followed by CONTINUATION frames carries the END_STREAM flag that signals the end of a stream. A CONTINUATION frame cannot be used to terminate a stream." from 6.2 is unclearly worded. The phrase "This unsigned 31-bit integer identifies the stream the endpoint intends to start sending frames for." (6.6) would make my grammar school teacher shudder. ;-) In 6.9.1, "For flow control calculations, the 8 byte frame header is not counted." should be octets. In 7, "Error codes share a common code space. Some error codes apply only to either streams or the entire connection and have no defined semantics in the other context." I would suggest re-wording as this could be misread. In 8.3, "The HTTP header field mapping works as mostly as defined in Request Header Fields (Section 8.1.2.3), with a few differences." Typo? - Robby Robby Simpson, PhD System Architect GE Digital Energy
Received on Monday, 4 August 2014 19:34:59 UTC