Subject: Misc Comments on Layering layering work and sections 1-5.

From: Mike Belshe <[email-masked]>
Date: Fri, 5 Jul 2013 08:37:04 -0700

Here are a bunch of comments on the latest spec.  Some of these are really
minor.  Let me know the best way to provide this feedback.

2 high-level bits about the layering:

a)  State diagram for streams.
This is a good diagram.  Suggestion:  Remove the "idle" state.  The diagram
(and text) states that all states that could ever exist are "idle", which
seems like they exist somehow.  So instead of starting in idle, why not
just depict 4 entry points into the state machine?
   a) PP send -> takes you to reserved (local)
   b) PP receive -> takes you to reserved (remote)
   c) H send -> takes you to open
   d) H receive -> takes you to open

b) Now that we're getting the stream lifecycle clean again, it will soon be
a good time to rethink the naming.  When you're new to the protocol, its
not clear that "HEADERS" is a way to start a stream.  START_STREAM would be
cleaner.  Similarly, PUSH_PROMISE requires a lot of knowledge and doesn't
sound stream-ish.  PROMISE_STREAM would be better.

Sorry to bring up naming - but I do think this is extremely important for
making the protocol easy to understand when folks read it for the first
time.

Section by section:

2.  I would strike all this text- its redundant with text in the
subsections and not really clarifying.

3 - 3.5.  The term "Starting HTTP/2.0" is understandable, but the document
lacks a basic "connection establishment" section.  I would probably rework
this to be "3. Connection management", and reword everything from that
viewpoint.

4.1.  In an effort to modernize protocol docs, I'd define in the
definitions a "byte" as a "sequence of 8 bits", and then strike the word
"octets" everywhere from the doc in favor of "bytes".

4.2.
  * Strike the note about PING, or move it to the section that deals with
PING.
  * I didn't understand when this happens: "If a frame size exceeds any
defined limit, or is too small to contain mandatory frame data"  I'd strike
this.

4.3
When I got to this section, it was very hard to find where we define what a
header block looks like!  We link to PP frames, error states, compression,
etc.  Section 6.2 refers back to 4.3 to define a header block, but neither
section actually defines a header block!  :-)

5.1
* We might reword the way we introduce the concept of 'associated stream'
here.
  Example
    "Sending a PUSH_PROMISE frame marks the associated stream..." .
  Perhaps change to:
   "Sending a PP frame creates a stream promised for future use.  Promised
streams ar always associated with an existing, open stream."

Editorial:  it saddens me that so much specification complexity is added to
the protocol for push when push is so secondary to the core protocol.

* Closed stream:
  This text:
  "An endpoint MUST NOT send frames on a closed stream. An endpoint that
receives a frame after receiving a RST_STREAM or a frame containing a
END_STREAM flag on that stream MUST treat that as a stream error (Section
5.4.2) of type STREAM_CLOSED."
   If we move it down one paragraph (after the description of how we
transition into it) can be simplified to:
  "An endpoint MUST NOT send frames on a closed stream.  An endpoint that
receives a frame while in the closed state MUST treat that as a stream
error of type STREAM_CLOSED."

5.4  I wondered if this section (Error handling, both stream & connection)
shouldn't be in section 4 (http framing), rather than section 5 (streams).
 Nothing seems to fit that well.

I stopped here for time....

Thanks everyone for the work on this.  I hope these comments are useful.

Mike

---

From: Amos Jeffries <[email-masked]>
Date: Sat, 06 Jul 2013 04:10:56 +1200

On 6/07/2013 3:37 a.m., Mike Belshe wrote:
> Here are a bunch of comments on the latest spec.  Some of these are 
> really minor.  Let me know the best way to provide this feedback.
>
> This is a good diagram.  Suggestion:  Remove the "idle" state.  The 
> diagram (and text) states that all states that could ever exist are 
> "idle", which seems like they exist somehow.  So instead of starting 
> in idle, why not just depict 4 entry points into the state machine?
> b) Now that we're getting the stream lifecycle clean again, it will 
> soon be a good time to rethink the naming.  When you're new to the 
> protocol, its not clear that "HEADERS" is a way to start a stream. 
>  START_STREAM would be cleaner.  Similarly, PUSH_PROMISE requires a 
> lot of knowledge and doesn't sound stream-ish.  PROMISE_STREAM would 
> be better.
>
> Sorry to bring up naming - but I do think this is extremely important 
> for making the protocol easy to understand when folks read it for the 
> first time.

Since this is being built as an upgrade on HTTP/1 and many implementers 
will have an HTTP/1 terminology background it would seem easier to use 
that terminology for these things. The suggested "START_STREAM" was once 
called "request headers" or "reply headers" depending on direction. It 
is now combined into one frame type for both directions so perhapse 
dropping the initial word and just calling it a HEADERS will be more 
familiar to implementers, yes?

PUSH_PROMISE is a new term without equivalence and all the talk so far 
has been about it under that name - so why change? particularly if that 
new scary "STREAM" word is halted at the doorway from SPDY world.

> 5.1
> * We might reword the way we introduce the concept of 'associated 
> stream' here.
>   Example
>    "Sending a PP frame creates a stream promised for future use. 
>  Promised streams ar always associated with an existing, open stream."
>
> Editorial:  it saddens me that so much specification complexity is 
> added to the protocol for push when push is so secondary to the core 
> protocol.

Ditto. Can PUSH be cleanly separated from the core protocol and made a 
second document?

Amos

---

From: Mike Belshe <[email-masked]>
Date: Fri, 5 Jul 2013 10:19:47 -0700

On Fri, Jul 5, 2013 at 9:10 AM, Amos Jeffries <[email-masked]> wrote:

>> diagram (and text) states that all states that could ever exist are "idle",
>> which seems like they exist somehow.  So instead of starting in idle, why
>> not just depict 4 entry points into the state machine?
>> b) Now that we're getting the stream lifecycle clean again, it will soon
>> be a good time to rethink the naming.  When you're new to the protocol, its
> will have an HTTP/1 terminology background it would seem easier to use that
> terminology for these things. The suggested "START_STREAM" was once called
> "request headers" or "reply headers" depending on direction. It is now
> combined into one frame type for both directions so perhapse dropping the
> initial word and just calling it a HEADERS will be more familiar to
> implementers, yes?
>

I'm looking for a name that indicates "beginning" or "start".  HTTP/1
doesn't have framing, of course, but does offer "Section 5. Request".  It's
a logical start point -- make a request.

>
> PUSH_PROMISE is a new term without equivalence and all the talk so far has
> been about it under that name - so why change? particularly if that new
> scary "STREAM" word is halted at the doorway from SPDY world.

I don't understand - the HTTP/2 spec uses "stream" pervasively.  It's not a
spdy remnant - its a practicality of HTTP/2.

Both PUSH_PROMISE and HEADERS are mechanisms to start streams, the core of
HTTP2.  The reason for change is because we often talk about "on stream"
concepts, and

Mike

>> Editorial:  it saddens me that so much specification complexity is added
>> to the protocol for push when push is so secondary to the core protocol.
>>
>

---

From: Michael Sweet <[email-masked]>
Date: Fri, 05 Jul 2013 14:31:48 -0400

Mike/Amos,

On 2013-07-05, at 1:19 PM, Mike Belshe <[email-masked]> wrote:
> ...
> Since this is being built as an upgrade on HTTP/1 and many implementers will have an HTTP/1 terminology background it would seem easier to use that terminology for these things. The suggested "START_STREAM" was once called "request headers" or "reply headers" depending on direction. It is now combined into one frame type for both directions so perhapse dropping the initial word and just calling it a HEADERS will be more familiar to implementers, yes?
> 
> I'm looking for a name that indicates "beginning" or "start".  HTTP/1 doesn't have framing, of course, but does offer "Section 5. Request".  It's a logical start point -- make a request.  

Something that's bugged me a little is that the context/directionality of the HEADERS frame type - you make certain assumptions that if you receive a HEADERS frame then you work with one header table and if you send a HEADERS frame then you work with a different table.  Whether you are receiving or sending request or response headers is implied by your role in a connection, and that is state you need to maintain and use in your implementation and clearly document, particularly in any generic client/server code that is shared between the two sides.

What if instead we had two frame types: REQUEST_HEADERS and RESPONSE_HEADERS?  Both would carry headers like the current HEADERS frame type, but the type of headers would be clearly defined by the frame type and perhaps be easier/less error prone to understand and implement.

(I don't see a need for two DATA frame types - DATA always follows headers and just depends on direction and the request/response type...)

_________________________________________________________
Michael Sweet, Senior Printing System Engineer, PWG Chair

---

From: Mike Belshe <[email-masked]>
Date: Fri, 5 Jul 2013 11:48:40 -0700

On Fri, Jul 5, 2013 at 11:31 AM, Michael Sweet <[email-masked]> wrote:

> Something that's bugged me a little is that the context/directionality of
> the HEADERS frame type - you make certain assumptions that if you receive a
> HEADERS frame then you work with one header table and if you send a HEADERS
> frame then you work with a different table.  Whether you are receiving or
> sending request or response headers is implied by your role in a
> connection, and that is state you need to maintain and use in your
> implementation and clearly document, particularly in any generic
> client/server code that is shared between the two sides.
>
> What if instead we had two frame types: REQUEST_HEADERS and
> RESPONSE_HEADERS?  Both would carry headers like the current HEADERS frame
> type, but the type of headers would be clearly defined by the frame type
> and perhaps be easier/less error prone to understand and implement.
>

Historically, the reason we didn't go this way before is because the
framing layer has no request/response requirement by itself.  However,  I
realize HTTP is request/response (push excluded), and we're not building a
generic framing layer.

Net result:  I do agree your naming is clearer than the current 'headers'
frame, so I like that angle.  It would also make pushed streams more
self-documenting too - as receiving a response_header block makes it clear
what the push is, even though there was no formal request.

> (I don't see a need for two DATA frame types - DATA always follows headers
> and just depends on direction and the request/response type...)
>

No - this is different.  We apply compression to the headers that doesn't
make sense on the data blocks.  Whether this is differentiated by frame
type or something else, its key.

Mike

---

From: Martin Thomson <[email-masked]>
Date: Fri, 5 Jul 2013 12:00:33 -0700

On 5 July 2013 11:48, Mike Belshe <[email-masked]> wrote:
>> type, but the type of headers would be clearly defined by the frame type and
>> perhaps be easier/less error prone to understand and implement.

I don't see a real advantage to this.  An endpoint knows its role.  It
can perform a lookup: initial_headers_table[myrole].

A more advanced approach might be to use a setting to describe which
initial headers table to use.  But I think that we've concluded that
this is better negotiated during the TLS handshake (that is, negotiate
a new protocol).

> I realize
> HTTP is request/response (push excluded), and we're not building a generic
> framing layer.

Push is really request/response too :)  The request is explicit too,
it just doesn't originate at the client.

---

From: Jeff Pinner <[email-masked]>
Date: Mon, 8 Jul 2013 10:51:40 -0700

Mike, thanks for looking over the draft. These suggestions mirror a few of
my own and seem mostly editorial.

I think the best way to address these at this point, and feel free to
disagree, is to merge streaming into master and release draft-04 for
implementation, then open these as new issues against draft-04.

---

From: Mike Belshe <[email-masked]>
Date: Mon, 8 Jul 2013 11:38:05 -0700

sgtm.