- From: Alan Egerton <eggyal@gmail.com>
- Date: Wed, 23 Sep 2020 02:03:57 +0100
- To: Mike Bishop <mbishop@evequefou.be>
- Cc: Lucas Pardue <lucaspardue.24.7@gmail.com>, HTTP Working Group <ietf-http-wg@w3.org>
- Message-ID: <CA+phaeesjdoTz7H1PvHdE3bYS9zip5OK2baOWNZKs+3yTDDxfw@mail.gmail.com>
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 > >
Received on Wednesday, 23 September 2020 01:04:22 UTC