Re: Stream State and PRIORITY Frames

On Tue, Jan 17, 2017 at 4:42 AM, Cory Benfield <cory@lukasa.co.uk> wrote:

>
> On 17 Jan 2017, at 08:38, Benedikt Christoph Wolters <
> benedikt.wolters@rwth-aachen.de> wrote:
>
> This question reminds me of a similiar issue we had a while ago at
> mitmproxy: https://github.com/mitmproxy/mitmproxy/issues/1498
>
> As far as I understand this, sending PRIORITY does not initiate a
> stream or change the stream state.
> HEADERS and PUSH_PROMISE initiate a stream. PRIORITY can be sent and
> received in any stream state.
>
>
> In fact, I think it’s the same issue. Scott linked to https://github.com/
> summerwind/h2spec/pull/67, which is referenced from and opened by the
> same person who opened https://github.com/h2o/h2o/pull/1136, which links
> to https://github.com/mitmproxy/mitmproxy/issues/1824, which is related
> to https://github.com/mitmproxy/mitmproxy/issues/1498 because both cases
> were sites deployed behind Fastly.
>
> For those who want background and don’t want to follow all those links,
> mitmproxy bumped into problems with sites deployed behind Fastly. mitmproxy
> encountered these because it was forwarding on traffic from Firefox, which
> would on connection-setup send a whole bunch of PRIORITY frames. hyper-h2
> (which powers mitmproxy’s HTTP/2 support) would allow this. h2o would not.
>
> The crux of the discussion boils down to a few apparently contradictory
> sections in the specification. Scott linked to the first two:
>
> Section 5.1.1:
>
> - The first use of a new stream identifier implicitly closes all streams
> in the “idle” state that may have been initiated by that peer with a
> lower-valued stream identifier.
> - An endpoint that receives an unexpected stream identifier must respond
> with a connection error of type PROTOCOL_ERROR.
>
> *However*, there are a few other notes. The first thing to note is that
> the PRIORITY frame does not transition a stream out of the idle state. A
> stream that has only had PRIORITY frames sent on it is still in the IDLE
> state. That means that the Firefox-style priority setup (sending priority
> for stream IDs 1, 3, 5, and 7 immediate after the preamble) would have the
> effect of immediately closing streams 1, 3, and 5, if section 5.1.1
> applied. This is not what most people expect.
>
>
This is interesting. If FF is using streams 1-7 then as soon as stream 9 is
used for a HEADERS request that will close streams 1-7 according to section
5.1.1:

> first use of a new stream identifier implicitly closes all streams in the
"idle" state that might have been initiated by that peer with a
lower-valued stream identifier

If these streams are not CLOSED, then what is the purpose for this special
exception? They are not allowed to transition state at a later point in
time because of 5.1.1:

> newly established stream MUST be numerically greater

"Option 1" requires special exception for stream ID allocation to
accommodate "PRIORITY streams remaining in IDLE" and I don't see the
benefit. The specification already allows (and even suggests) retaining
priority state for CLOSED streams in section 5.3.4:

> If it has retained enough state to do so, an endpoint receiving a PRIORITY
frame that changes the priority of a closed stream SHOULD alter the
dependencies of the streams that depend on it.

So in terms of priority just because a stream is CLOSED doesn't necessarily
mean any priority information is lost. Even if the streams remain in IDLE
implementations will have to impose some limit on how much memory is can be
consumed by these "PRIORITY streams in IDLE" so it just seems like
additional complexity to make this exception.


> More importantly, though, section 5.1.1 says this:
>
> - The identifier of a newly established stream MUST be numerically greater
> than all streams that the initiating endpoint has opened or reserved.
> This governs streams that are opened using a HEADERS frame and streams that
> are reserved using PUSH_PROMISE.
>
> This is not absolutely clear, but it seems to suggest that implementations
> may establish a stream with an ID lower than streams that have had PRIORITY
> frames sent on them. Put another way, it seems to suggest that only HEADERS
> and PUSH_PROMISE frames affect the lowest-acceptable stream ID. That seems
> to me to suggest that PRIORITY frames are not expected to force-close all
> streams with lower IDs that are in the idle state.
>
> As an implementers note, I have found it much more helpful to think of
> PRIORITY frames not as frames that are sent on a stream, but as frames that
> are implicitly sent on stream zero and are just tagged with a stream ID.
> PRIORITY frames ignore basically all restrictions on stream behaviour: they
> can be sent on idle streams, on closed streams. In practice, they can
> basically be sent whenever, and except for the case where they establish a
> loop in the priority tree must basically always be accepted.
>
> Cory
>