Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signal error upon receipt of multiple trailing HEADERS #2858

Closed
bencebeky opened this issue Jun 28, 2019 · 3 comments
Closed

Signal error upon receipt of multiple trailing HEADERS #2858

bencebeky opened this issue Jun 28, 2019 · 3 comments
Labels
-http design An issue that affects the design of the protocol; resolution requires consensus.

Comments

@bencebeky
Copy link
Contributor

According to https://quicwg.org/base-drafts/draft-ietf-quic-http.html#request-response, a bidirectional HTTP stream can have at least one HEADERS frame (exactly one if the stream is client-initiated), zero or more DATA frames, then zero or one HEADERS frame, in this order. Handling the receipt of frames that does not conform to this requirement is inconsistent:

(1) If a DATA frame is received before any other frame, that's an error:

If a DATA frame is received before a HEADERS frame [...], the recipient MUST respond with a connection error of type HTTP_UNEXPECTED_FRAME [...].

(2) If the trailing HEADERS frame is followed by another HEADERS frame, that is to be discarded:

Trailing header fields are carried in an additional HEADERS frame following the body. Senders MUST send only one HEADERS frame in the trailers section; receivers MUST discard any subsequent HEADERS frames.

(3) There is no guidance on how to handle a DATA frame after the trailing headers.

I suggest that every violation to this requirement should be an error. (I do not feel strongly whether a connection error like (1) is handled right now, or a stream error. The implementation I am contributing to will promote most stream errors to connection errors regardless.)

Note that (2) comes from the technical limitation that header blocks needed to be processed, which is not the case any longer with QPACK. It just seems like that when the requirement to decode the header block was removed at 404c5b8, adding the requirement to signal an error was not in scope of that commit.

@LPardue
Copy link
Member

LPardue commented Jun 28, 2019

I agree with @bencebeky .

If a DATA frame is received before a HEADERS frame [...], the recipient MUST respond with a connection error of type HTTP_UNEXPECTED_FRAME [...].

If you read that requirement a certain way, then any use of trailers will blow up the connection :) I think we could rewrite this text to be clearer about when DATA frames are expected to be received.

@MikeBishop MikeBishop added -http design An issue that affects the design of the protocol; resolution requires consensus. labels Jun 28, 2019
@MikeBishop
Copy link
Contributor

Good points. Let's continue tightening down the screws. :-)

DaanDeMeyer pushed a commit to DaanDeMeyer/google-quiche-iquic that referenced this issue Aug 1, 2019
While the HTTP/3 spec draft does not require this yet, this is the sane thing to
do, and my proposal to change the spec has some support:
quicwg/base-drafts#2858.

This fixes fuzzer-found bug https://crbug.com/978733.

NO_BUG=Bug is https://crbug.com/978733 on Chromium bugtracker.

gfe-relnote: n/a, change in QUIC v99-only code.
PiperOrigin-RevId: 257169271
Change-Id: I10ed66e671f0dc03d6f24694d4c53ba3691e9c51
MikeBishop pushed a commit that referenced this issue Aug 2, 2019
* Tighten HEADERS and DATA frame order requirements.

* Clarify HTTP message definition.
* Require the connection to be closed upon receipt of frames in
  incorrect order.
* Refer to HTTP messages in push stream section.

This addresses #2858.

* Do not exclude extension frames on push streams.

* Explicitly allow unknown frames.

* wordsmithing
@MikeBishop
Copy link
Contributor

Closed in #2867.

gcjenkinson pushed a commit to chromium-cheri/quiche that referenced this issue Apr 22, 2024
While the HTTP/3 spec draft does not require this yet, this is the sane thing to
do, and my proposal to change the spec has some support:
quicwg/base-drafts#2858.

This fixes fuzzer-found bug https://crbug.com/978733.

NO_BUG=Bug is https://crbug.com/978733 on Chromium bugtracker.

gfe-relnote: n/a, change in QUIC v99-only code.
PiperOrigin-RevId: 257169271
Change-Id: I10ed66e671f0dc03d6f24694d4c53ba3691e9c51
gcjenkinson pushed a commit to chromium-cheri/quiche that referenced this issue Apr 22, 2024
While the HTTP/3 spec draft does not require this yet, this is the sane thing to
do, and my proposal to change the spec has some support:
quicwg/base-drafts#2858.

This fixes fuzzer-found bug https://crbug.com/978733.

NO_BUG=Bug is https://crbug.com/978733 on Chromium bugtracker.

gfe-relnote: n/a, change in QUIC v99-only code.
PiperOrigin-RevId: 257169271
Change-Id: I10ed66e671f0dc03d6f24694d4c53ba3691e9c51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

No branches or pull requests

3 participants