Re: HTTP/2 client behaviour on receiving illegal PUSH_PROMISE frames

Ok, thanks for the clarification.  I hadn't considered that the spec might
be requiring multiple errors be raised/handled.

-- Alan


On Tue, Sep 22, 2020 at 11:10 PM Mike Bishop <mbishop@evequefou.be> wrote:

> Section 6.6 says that you C:P-E (using your notation) for receipt of
> PUSH_PROMISE when they can’t have legally been sent; it has an exception to
> account for a race condition, and says that you “MUST handle” them.  So it
> doesn’t mandate anything in particular beyond handling as normal in that
> race condition.  I’d echo most of Lucas’s points; it doesn’t need to
> specify the error code for RST_STREAM, because you’re “handling” it
> normally.  If you don’t want it, use the error code you would routinely use
> for unwanted pushes.
>
>
>
> However, you’re correct that receiving a PUSH_PROMISE in “half-closed
> (remote)” isn’t one of these race conditions; that means that the exception
> doesn’t apply in that case and the requirement to C:P-E applies in that
> scenario.
>
>
>
> It’s a bit odd that we require *both* a stream error *and *a connection
> error triggered by the same frame, but the ordering of those doesn’t
> matter; the connection is closed at the end either way.  I suppose my
> lawyerly answer is that a requirement to generate a stream error doesn’t
> preclude also treating it as a connection error, so this is merely a bit
> awkward and not outright wrong.
>
>
>
> *From:* Lucas Pardue <lucaspardue.24.7@gmail.com>
> *Sent:* Tuesday, September 22, 2020 5:03 PM
> *To:* Alan Egerton <eggyal@gmail.com>
> *Cc:* HTTP Working Group <ietf-http-wg@w3.org>
> *Subject:* Re: HTTP/2 client behaviour on receiving illegal PUSH_PROMISE
> frames
>
>
>
> My read is that 6.6 is a specialisation of the error conditions described
> in 5.1, therefore it might help to mention that in 5.1. Others might
> disagree.
>
>
>
> As for RST_STREAM of the unwanted push, I'd say this is just as described
> in 8.2.2; either CANCEL or REFUSED_STREAM.
>
>
>
> On Tue, 22 Sep 2020, 21:42 Alan Egerton, <eggyal@gmail.com> wrote:
>
> Hi Lucas,
>
>
> The situations I've described are absolutely illegal PUSH_PROMISE
> frames—but sections 5.1 and 6.6 appear to disagree over what error handling
> is appropriate in those situations.
>
>
> -- Alan
>
>
>
>
>
> On Tue, Sep 22, 2020 at 9:09 PM Lucas Pardue <lucaspardue.24.7@gmail.com>
> wrote:
>
>
>
>
>
> On Tue, Sep 22, 2020 at 8:38 PM Alan Egerton <eggyal@gmail.com> wrote:
>
> Thanks Lucas, but I don't think either of those reports are quite the
> same: they both appear to concern the state transition from "idle" state on
> sending a PUSH_PROMISE (and that the spec can be misread as describing
> transitions from that state on receiving such frames); whereas I am
> concerned with the correct error handling on receiving an erroneous
> PUSH_PROMISE in the "half-closed(remote)" or "closed" states.
>
>
>
> -- Alan
>
>
>
>
>
> oops I meant to say "possibly related" because this is about the handling
> of push promises with respect to stream lifecycle. My 2c:
>
>
>
> I might be squinting at the state machine wrong but I don't think it is
> practically possible for the client to have a request stream in a
> half-closed (remote) and receive a PUSH_PROMISE. Because the only way to
> get a stream in that state is for a server to respond to a request with
> END_STREAM set, before the client has sent END_STREAM or RST_STREAM. This
> is an early response, which is allowed. But the server shouldn't be trying
> to promise things after it closed the stream, that's a plain error.
> Similarly, a server sending PUSH_PROMISE after RST_STREAM is also an error.
>
>
>
> The odd case is when a client and server have a race about the stream
> being closed due to the client sending RST_STREAM in the open state.
> "Closed because I said so" is a bit different to "Closed because you said
> so". The statement in 6.6 about "MUST handle PUSH_PROMISES" is trying to
> wiggle out of the race condition.
>
>
>
> Cheers
>
> Lucas
>
>