draft-ietf-httpbis-http2 feedback

[Julian Reschke <[e-mail]>]

Questions:

In 4.1:

"Type:
     The 8-bit type of the frame. The frame type determines how the 
remainder of the frame header and payload are interpreted. 
Implementations MUST ignore unsupported and unrecognized frame types."

Maybe s/unrecognized frame types/frames of unsupported or unrecognized 
types/?

(It's the frame that is ignored, right?)


6.3

"The PRIORITY frame (type=0x2) specifies the sender-advised priority of 
a stream. It can be sent at any time for an existing stream. This 
enables reprioritisation of existing streams."

Can it interleave HEADERS frames?

6.8

"Endpoints MAY append opaque data to the payload of any GOAWAY frame. 
Additional debug data is intended for diagnostic purposes only and 
carries no semantic value. Debug data MUST NOT be persistently stored, 
since it could contain sensitive information."

Does this include loggers / trace tools??? If they don't store the 
information, how can it be used for debugging?


6.9.4 Ending Flow Control

"Flow control cannot be enabled again once disabled."

I assume there's a reason for that. Maybe it's worth explaining?


8.1 HTTP Request/Response Exchange

Why are method names being lowercased??????


8.1.2 Request Header Fields

"All header field names MUST be lowercased, and the definitions of all 
header field names defined by HTTP/1.1 are updated to be all lowercase."

I don't understand the second part of the sentence.

"All HTTP Requests that include a body SHOULD include the 
"content-length" header field."

Why? If I don't know the size upfront and leave it out, am I violating a 
requirement here? (I believe this is "SHOULD/MUST if known upfront").


8.1.3 Response Header Fields

Dropping the reason-phrase might break a few edge cases; is this worth 
it????


8.2

Is it a good idea to completely rule out pushed responses to methods != GET?


Editorial:

Throughout: I really dislike the use of "zero or more". But maybe it's 
just me :-)


"1.1 Document Organization

The HTTP/2.0 Specification is split into three parts: starting HTTP/2.0 
(Section 3), which covers how a HTTP/2.0 connection is initiated; a 
framing layer (Section 4), which multiplexes a single TCP connection 
into independent frames of various types; and an HTTP layer (Section 8), 
which specifies the mechanism for expressing HTTP interactions using the 
framing layer. While some of the framing layer concepts are isolated 
from HTTP, building a generic framing layer has not been a goal. The 
framing layer is tailored to the needs of the HTTP protocol and server 
push."

So we have forward references to Sections 3, 4 and 8. But not 5, 6, or 7?


"2.3 HTTP Semantics

HTTP/2.0 defines how HTTP requests and responses are mapped to streams 
(see Section 8) and introduces a new interaction model, server push 
(Section 8.2)."

The first reference probably should point to Section 8.1.


"3.5 Connection Header

Upon establishment of a TCP connection and determination that HTTP/2.0 
will be used by both peers, each endpoint MUST send a connection header 
as a final confirmation and to establish the initial settings for the 
HTTP/2.0 connection.

The client connection header is a sequence of 24 octets, which in hex 
notation are:

505249202a20485454502f322e300d0a0d0a534d0d0a0d0a

(the string PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n) followed by a SETTINGS 
frame (Section 6.5). "

I believe it would be clearer if we said:

"3.5 Connection Header

Upon establishment of a TCP connection and determination that HTTP/2.0 
will be used by both peers, each endpoint MUST send a connection header 
as a final confirmation and to establish the initial settings for the 
HTTP/2.0 connection.

The client connection header is a sequence of 24 octets, which in hex 
notation are:

	505249202a20485454502f322e300d0a0d0a534d0d0a0d0a

This happens to be the HTTP message

   PRI * HTTP/2.0
   SM

This octet sequence is followed by a SETTINGS frame (Section 6.5)."


"4.1 Frame Header

All frames begin with an 8-octet header followed by a payload of between 
0 and 65,535 octets.

   0                   1                   2                   3
   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |         Length (16)           |   Type (8)    |   Flags (8)   |
  +-+-------------+---------------+-------------------------------+
  |R|                 Stream Identifier (31)                      |
  +-+-------------------------------------------------------------+
  |                   Frame Payload (0...)                      ...
  +---------------------------------------------------------------+"

I found it confusing that the section is called "Frame Header" but then 
defines the format of the complete frame.

Maybe:

"4.1 Frame Format

All frames begin with an 8-octet header followed by a payload of between 
0 and 65,535 octets.

   0                   1                   2                   3
   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |         Length (16)           |   Type (8)    |   Flags (8)   |
  +-+-------------+---------------+-------------------------------+
  |R|                 Stream Identifier (31)                      |
  +-+-------------------------------------------------------------+
  +---------------------------------------------------------------+
  |                   Frame Payload (0...)                      ...
  +---------------------------------------------------------------+"


5.1

Maybe prefix the diagram with a list of states and their abbreviations.


5.2.2

"Deployments with constrained resources (for example, memory) MAY employ 
flow control to limit the amount of memory a peer can consume. Note, 
however, that this can lead to suboptimal use of available network 
resources if flow control is enabled without knowledge of the 
bandwidth-delay product (see [RFC1323])."

s/MAY/can/


6.2

"The HEADERS frame (type=0x1) carries name-value pairs. The HEADERS is 
used to open a stream (Section 5.1). Any number of HEADERS frames can be 
sent on an existing stream at any time."

s/The HEADERS is used/It is used/

"PRIORITY (0x8):
     Bit 4 being set indicates that the first four octets of this frame 
contain a single reserved bit and a 31-bit priority; see Section 5.3. If 
this bit is not set, the four bytes do not appear and the frame only 
contains a header block fragment."

May rename PRIORITY to HAS_PRIORITY then?


6.5.2 Defined Settings

Is there a reason for the numerical constants being 4..7..10?


6.8

"On streams with lower or equal numbered identifiers that were not 
closed completely prior to the connection being closed, re-attempting 
requests, transactions, or any protocol activity is not possible (with 
the exception of idempotent actions like HTTP GET, PUT, or DELETE). Any 
protocol activity that uses higher numbered streams can be safely 
retried using a new connection."

Add a ref to definition of idempotent.


8.2

s/PUSH_PROMSE/PUSH_PROMISE/


IANA Considerations

Should either list the things to be registered, or reference them by 
section #.



Best regards, Julian