Subject: draft-ietf-httpbis-http2-08 / 5.1. Stream States

From: Kari Hurtta <[email-masked]>
Date: Sat, 30 Nov 2013 09:30:51 +0000

http://tools.ietf.org/html/draft-ietf-httpbis-http2-08

On chapter  "5.1. Stream States"  state "reserved (local):"
     
there is

|      A PRIORITY frame MAY be received in this state.  Receiving any
|      frame other than HEADERS, RST_STREAM, or PRIORITY MUST be treated
|      as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
 
Should this sentence be

       "Receiving any frame other than RST_STREAM, or PRIORITY MUST be
       treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR."

Note that on state "reserved (remote):" there is

|      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.
 
Because "reserved (remote):" state does not allow sending HEADERS frame,
then "reserved (local):" state should not receive HEADERS frame.

Or what I have missing ?

/ Kari Hurtta

---

From: Martin Thomson <[email-masked]>
Date: Mon, 2 Dec 2013 10:53:22 -0800

>        treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR."

Yes, you are correct.

https://github.com/http2/http2-spec/commit/3d0b6449425b7b7f1c0314136d323f9c019d913a

---

From: Martin Thomson <[email-masked]>
Date: Thu, 19 Dec 2013 10:20:22 -0800

On 19 December 2013 09:08, Kari Hurtta <[email-masked]> wrote:
> 6.6 PUSH_PROMISE
>
> | Similarly, a receiver MUST treat the receipt of a PUSH_PROMISE that
> | promises an illegal stream identifier (Section 5.1.1) (that is, an
> | identifier for a stream that is not currently in the "idle" state) as
> | a connection error (Section 5.4.1) of type PROTOCOL_ERROR, unless the
> | receiver recently sent a RST_STREAM frame to cancel the associated
> | stream (see Section 5.1).
>
> If recipient is sent RST_STREAM then it goes to "closed" state, not
> to "idle" state.

I think that the "unless..." here offers an option that doesn't make
sense.  The original problem, if I remember correctly, was that there
was a chance that a promise could arrive on a stream that was reset.

We deal with that elsewhere already:

"""
An endpoint might receive a PUSH_PROMISE frame after it sends
RST_STREAM. PUSH_PROMISE causes a stream to become "reserved". The
RST_STREAM does not cancel any promised stream. Therefore, if promised
streams are not desired, a RST_STREAM can be used to close any of
those streams.
"""

Thus, I'm going to remove the "unless..." here and avoid confusion.
While I'm there, I'll clarify that latter statement, because - having
now re-read it - it's a little ambiguous.

---

From: Martin Thomson <[email-masked]>
Date: Thu, 19 Dec 2013 10:38:16 -0800

> now re-read it - it's a little ambiguous.

See: https://github.com/http2/http2-spec/commit/9ee41297c4a5b05288847a45404b4741b2ff5b8d

---

From: Kari Hurtta <[email-masked]>
Date: Thu, 19 Dec 2013 17:09:16 +0000

> https://github.com/http2/http2-spec/commit/3d0b6449425b7b7f1c0314136d323f9c019d913a

Possible other.

http://http2.github.io/http2-spec/#PUSH_PROMISE

6.6 PUSH_PROMISE

So I do not understand why there is "unless the receiver recently sent a
RST_STREAM frame to cancel the associated stream (see Section 5.1)."

http://http2.github.io/http2-spec/#RST_STREAM

6.4 RST_STREAM

| The RST_STREAM frame fully terminates the referenced stream and causes
| it to enter the closed state. 

And there is also:

| RST_STREAM frames MUST NOT be sent for a stream in the "idle" state.
| If a RST_STREAM frame identifying an idle stream is received, the
| recipient MUST treat this as a connection error (Section 5.4.1) of
| type PROTOCOL_ERROR.

So that associated stream can not have be on "idle" state either 
before recipient of PUSH_PROMISE was sent RST_STREAM.

What I have missing ?

/ Kari Hurtta

---

From: Martin Thomson <[email-masked]>
Date: Mon, 23 Dec 2013 09:10:17 -0800

On 21 December 2013 03:08, Kari Hurtta <[email-masked]> wrote:
>   HTTP/1.1 200 OK               HEADERS
>   Content-Type: image/jpeg ===>   - END_STREAM
>   Content-Length: 123             + END_HEADERS
>   Transfer-Encoding: chunked        :status        = 200
>   TE: trailers                      content-length = 123
>   123                               content-type   = image/jpeg
>   {binary data}
>   0                             DATA
>   Foo: bar                        - END_STREAM
>                                     {binary data}
>
>                                 HEADERS
>                                   + END_STREAM
>                                   + END_HEADERS
>                                     foo: bar

Yes, this is in error.  content-length is always valid in HTTP/2.0,
but not in 1.1 when there is Transfer-Encoding.

https://github.com/http2/http2-spec/commit/6cf73458aef62169db4a3a65fe520111a71f1d78