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

[Mike Bishop <[e-mail]>]

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 <[e-mail]> 
Sent: Tuesday, September 22, 2020 5:03 PM
To: Alan Egerton <[e-mail]>
Cc: HTTP Working Group <[e-mail]>
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, <[e-mail] <mailto:[e-mail]> > 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 <[e-mail] <mailto:[e-mail]> > wrote:

 

 

On Tue, Sep 22, 2020 at 8:38 PM Alan Egerton <[e-mail] <mailto:[e-mail]> > 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