Subject: Rationalise HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME

From: Lucas Pardue <[email-masked]>
Date: Wed, 7 Aug 2019 01:02:55 +0100

Hello WG,

As part of the HTTP error analysis and feedback, in June I created issue
#2809 "Rationalise HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME" [1].

In short, HTTP_WRONG_STREAM is used for connection errors when a
non-allowed frame is received by a peer (e.g a server receiving
PUSH_PROMISE), or when a request sequence is malformed.
HTTP_UNEXPECTED_FRAME is used for connection errors when an allowed frame
is received on the wrong type of stream (e.g. a client receiving
PUSH_PROMISE on the control stream).

Issue 2809 suggests combining these error codes but I've yet to make a PR.
In the meantime, a fair amount of error code refactoring has taken place
and I'm not sure how much demand there is for the change.

If you have an opinion on this I'd appreciate feedback here or on the
ticket.

Cheers
Lucas

[1] https://github.com/quicwg/base-drafts/issues/2809

---

From: "Martin Thomson" <[email-masked]>
Date: Wed, 07 Aug 2019 10:28:35 +1000

On Wed, Aug 7, 2019, at 10:03, Lucas Pardue wrote:
> As part of the HTTP error analysis and feedback, in June I created 
> issue #2809 "Rationalise HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME" 
> [1]. 

The notion of a signal for "I got a frame that I understood, but it was in the wrong place" is fine.  Good even.  But I don't see a need to have two such signals.  So I'm in favour of collapsing the two.  I don't understand the distinction between these signals as it stands anyway.

However, I'm also concerned that this is going to get wrapped up in the usual compliance test mess.  If I hook up different parsers on different streams and my request stream parser doesn't know about SETTINGS at all, so ignores it when it suddenly appears on a request stream, is the test case going to fail when I don't generate HTTP_FRAME_IN_THE_WRONG_PLACE?

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 7 Aug 2019 01:47:26 +0100

On Wed, Aug 7, 2019 at 1:29 AM Martin Thomson <[email-masked]> wrote:

>
> However, I'm also concerned that this is going to get wrapped up in the
> usual compliance test mess.  If I hook up different parsers on different
> streams and my request stream parser doesn't know about SETTINGS at all, so
> ignores it when it suddenly appears on a request stream, is the test case
> going to fail when I don't generate HTTP_FRAME_IN_THE_WRONG_PLACE?
>

My intention would be to keep the exisiting error rules and behaviour. Each
stream type has a set of allowed Frames, as outlined in Section 7 / Table
1. If your request stream parser totally ignores a mandatory HTTP/3 frame
like SETTINGS, it is presently non-conformant (for whatever you choose that
to mean).

---

From: Ian Swett <[email-masked]>
Date: Wed, 7 Aug 2019 09:26:36 -0400

The distinction seems fairly clear to me and I'm happy keeping the draft as
is.

On Tue, Aug 6, 2019 at 8:47 PM Lucas Pardue <[email-masked]>
wrote:

> My intention would be to keep the exisiting error rules and behaviour.
> Each stream type has a set of allowed Frames, as outlined in Section 7 /
> Table 1. If your request stream parser totally ignores a mandatory HTTP/3
> frame like SETTINGS, it is presently non-conformant (for whatever you
> choose that to mean).
>
>
>

---

From: Daan De Meyer <[email-masked]>
Date: Wed, 7 Aug 2019 16:05:17 +0200

(I accidentally only replied to Ian so I'm reposting to the entire list)

Is there a significant amount of added value by having both
HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME? If these errors will only
be used for debugging and compliance testing in practice, I think we
can definitely get away with just reporting HTTP_UNEXPECTED_FRAME
whenever a frame is received that isn't supposed to be received at
that point in the connection on that stream.

As a very minor nitpick, having only HTTP_UNEXPECTED_FRAME also
simplifies error handling in implementations as a default case can be
added on a switch on frame type that throws HTTP_UNEXPECTED_FRAME for
every frame type that isn't explicitly handled in the switch
statement. Currently, one has to differentiate between frame types
that should be reported as HTTP_WRONG_STREAM and frame types that
should be reported as HTTP_UNEXPECTED_FRAME.

My recommendation would be to replace all usages of HTTP_WRONG_STREAM
by HTTP_UNEXPECTED_FRAME as in my opinion having both does not add
significant value.

Regards,

Daan De Meyer

On Wed, 7 Aug 2019 at 02:03, Lucas Pardue <[email-masked]> wrote:
>
> Hello WG,
>
> As part of the HTTP error analysis and feedback, in June I created issue #2809 "Rationalise HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME" [1].
>
> In short, HTTP_WRONG_STREAM is used for connection errors when a non-allowed frame is received by a peer (e.g a server receiving PUSH_PROMISE), or when a request sequence is malformed. HTTP_UNEXPECTED_FRAME is used for connection errors when an allowed frame is received on the wrong type of stream (e.g. a client receiving PUSH_PROMISE on the control stream).
>
> Issue 2809 suggests combining these error codes but I've yet to make a PR. In the meantime, a fair amount of error code refactoring has taken place and I'm not sure how much demand there is for the change.
>
> If you have an opinion on this I'd appreciate feedback here or on the ticket.
>
> Cheers
> Lucas
>

---

From: Mike Bishop <[email-masked]>
Date: Thu, 8 Aug 2019 19:15:48 +0000

That would be a dangerous coding pattern.  You're required to ignore frames you don’t understand -- that's what allows unnegotiated extension frames.  These error codes are explicitly for frames that you *do* understand and therefore know that they shouldn't be used in the way they are.

-----Original Message-----
From: QUIC <[email-masked]> On Behalf Of Daan De Meyer
Sent: Wednesday, August 7, 2019 10:05 AM
To: Lucas Pardue <[email-masked]>
Cc: QUIC WG <[email-masked]>
Subject: Re: Rationalise HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME

Is there a significant amount of added value by having both HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME? If these errors will only be used for debugging and compliance testing in practice, I think we can definitely get away with just reporting HTTP_UNEXPECTED_FRAME whenever a frame is received that isn't supposed to be received at that point in the connection on that stream.

As a very minor nitpick, having only HTTP_UNEXPECTED_FRAME also simplifies error handling in implementations as a default case can be added on a switch on frame type that throws HTTP_UNEXPECTED_FRAME for every frame type that isn't explicitly handled in the switch statement. Currently, one has to differentiate between frame types that should be reported as HTTP_WRONG_STREAM and frame types that should be reported as HTTP_UNEXPECTED_FRAME.

My recommendation would be to replace all usages of HTTP_WRONG_STREAM by HTTP_UNEXPECTED_FRAME as in my opinion having both does not add significant value.

Regards,

Daan De Meyer

On Wed, 7 Aug 2019 at 02:03, Lucas Pardue <[email-masked]<mailto:[email-masked]>> wrote:

> Issue 2809 suggests combining these error codes but I've yet to make a PR.. In the meantime, a fair amount of error code refactoring has taken place and I'm not sure how much demand there is for the change.

---

From: Mike Bishop <[email-masked]>
Date: Thu, 8 Aug 2019 19:15:52 +0000

As Lucas said, if you have a separate parser by stream type and the frames defined for other streams simply aren't handled, that would violate the MUST-error requirements we currently have.  You would fail a compliance test.

I see a couple separable questions that we need to answer to move forward here:

  *   Should this even be possible?  QUIC has established a pattern of making it impossible to send invalid values.  An H3 incarnation of that pattern would be to split the frame type spaces so there's no such thing as WRONG_STREAM, just unknown frame types.  Should WRONG_STREAM be possible to express?
  *   If so, is this error easily detectable?  We've established a principle that easily detected protocol violations will be required to generate an error.  The current text reflects this principle, since known frame types and their valid streams are easily detectable.
     *   If the pattern Martin suggests might be common, and therefore this error isn't "easily detected," these would become SHOULD-error instead.
  *   Only then, the original question, is there value in having separate error codes?

I find the separate frame type registries slightly attractive, but unless we cross-reserve values (which defeats the point), we'd be breaking the last vestiges of HTTP/2 resemblance.  It's also a pretty fundamental change for a protocol on the edge of late-stage process.  I don't think I'd want to bite that off unless the working group as a whole feels strongly that it's an improvement, and I'm not seeing that so far.

I tend to think that misuse of known frame types is easily detectable and therefore Martin's hypothetical implementation is simply wrong.  However, the value in the distinction between the two seems minimal.  I'd vote for merging.

---

From: Daan De Meyer <[email-masked]>
Date: Thu, 8 Aug 2019 21:29:15 +0200

Of course, that's completely right. I forgot to mention that unknown
frame types are already ignored when I'm parsing frames so when my
implementation is at the point where it switches on frame type to
determine the next action to take, all unknown frame types have
already been ignored.

Regards,

Daan

---

From: Lucas Pardue <[email-masked]>
Date: Thu, 8 Aug 2019 22:08:04 +0100

I also overlooked something, HTTP_MALFORMED_FRAME was replaced with
HTTP_FRAME_ERROR which is presently defiend for use when a frame layout is
wrong, or its size is wrong.

Is the case of syntax error sufficiently unique from used incorrectly error
that HTTP_WRONG_STREAM and HTTP_UNEXPECTED_FRAME  currently articulate?
There is some overlap, so an more extreme step would be to merge them into
HTTP_FRAME_ERROR.

I'm undecided on the best course of action here, I don't even think
preparing candidate PRs will help the conversation much because of the
nature of the issue.

On Thu, Aug 8, 2019 at 8:16 PM Mike Bishop <[email-masked]> wrote:

> As Lucas said, if you have a separate parser by stream type and the frames
> defined for other streams simply aren't handled, that would violate the
> MUST-error requirements we currently have.  You would fail a compliance
> test.
>
>
>
>    - *Should this even be possible?*  QUIC has established a pattern of
>    making it impossible to send invalid values.  An H3 incarnation of that
>    pattern would be to split the frame type spaces so there’s no such thing as
>    WRONG_STREAM, just unknown frame types.  Should WRONG_STREAM be possible to
>    express?
>    - *If so, is this error easily detectable?*  We’ve established a
>    principle that easily detected protocol violations will be required to
>    generate an error.  The current text reflects this principle, since known
>    frame types and their valid streams are easily detectable.
>       - If the pattern Martin suggests might be common, and therefore
>       this error isn’t “easily detected,” these would become SHOULD-error instead.
>    - Only then, the original question, *is there value in having separate
>    error codes?*
>
>
>
> I find the separate frame type registries slightly attractive, but unless
> we cross-reserve values (which defeats the point), we’d be breaking the
> last vestiges of HTTP/2 resemblance.  It’s also a pretty fundamental change
> for a protocol on the edge of late-stage process.  I don’t think I’d want
> to bite that off unless the working group as a whole feels strongly that
> it’s an improvement, and I’m not seeing that so far.
>
>
>
> I tend to think that misuse of known frame types is easily detectable and
> therefore Martin’s hypothetical implementation is simply wrong.  However,
> the value in the distinction between the two seems minimal.  I’d vote for
> merging.
>
>
>
> The notion of a signal for "I got a frame that I understood, but it was in
> the wrong place" is fine.  Good even.  But I don't see a need to have two
> such signals.  So I'm in favour of collapsing the two.  I don't understand
> the distinction between these signals as it stands anyway.
>
>
>

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 9 Aug 2019 12:47:56 +0900

> As Lucas said, if you have a separate parser by stream type and the frames defined for other streams simply aren't handled, that would violate the MUST-error requirements we currently have.  You wo…

I think I tend to agree with Martin that this is an unnecessary
complexity, and that we should change the detection rule to SHOULD
(and also merge the two error codes to one).

While it is true that we have been trying to be strict in HTTP/3, I am
not sure if we are happy with enforcing every endpoint to detect a
misuse of a frame on a stream that it is never expected to appear
(e.g. receiving SETTINGS frame on a request stream).

Having such rules as a MUST does not improve interoperability. It's
purely about adding burden to the implementors.

As an example, I have about 50 lines of code that implements these
rules. I'd be in favor of removing these lines.
https://github.com/h2o/h2o/blob/dc12a571985e48b9725463f89888dd5a9d2571f4/lib/http3/common.c#L556-L603

> Should this even be possible?  QUIC has established a pattern of making it impossible to send invalid values.  An H3 incarnation of that pattern would be to split the frame type spaces so there’s no such thing as WRONG_STREAM, just unknown frame types.  Should WRONG_STREAM be possible to express?
> If so, is this error easily detectable?  We’ve established a principle that easily detected protocol violations will be required to generate an error.  The current text reflects this principle, since known frame types and their valid streams are easily detectable.
>
> If the pattern Martin suggests might be common, and therefore this error isn’t “easily detected,” these would become SHOULD-error instead.
>
> Only then, the original question, is there value in having separate error codes?
>
>
>
> I find the separate frame type registries slightly attractive, but unless we cross-reserve values (which defeats the point), we’d be breaking the last vestiges of HTTP/2 resemblance.  It’s also a pretty fundamental change for a protocol on the edge of late-stage process.  I don’t think I’d want to bite that off unless the working group as a whole feels strongly that it’s an improvement, and I’m not seeing that so far.
>
>
>
> I tend to think that misuse of known frame types is easily detectable and therefore Martin’s hypothetical implementation is simply wrong.  However, the value in the distinction between the two seems minimal.  I’d vote for merging.
>
>
>

-- 
Kazuho Oku

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 30 Aug 2019 21:01:49 +0100

For anyone not tracking the GitHub repo, I created a PR that collpses all
frame-related errors into HTTP_FRAME_ERROR in order to see if that moves
the needle for anyone.

https://github.com/quicwg/base-drafts/pull/2996

Lucas