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

http3: Are reserved and non-core frames allowed before SETTINGS? #2693

Closed
LPardue opened this issue May 12, 2019 · 5 comments · Fixed by #2997
Closed

http3: Are reserved and non-core frames allowed before SETTINGS? #2693

LPardue opened this issue May 12, 2019 · 5 comments · Fixed by #2997
Labels
-http editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@LPardue
Copy link
Member

LPardue commented May 12, 2019

We've had a long standing requirement that

A SETTINGS frame MUST be sent as the first frame of each control
stream (see Section 3.2.1) by each peer

By the way I interpret this, I put a check in my implementation that doesn't permit any frame until SETTINGS is received. Am I interpreting things correctly?

It would be more consistent to allow reserved frame types to appear at any point on any stream that carries frames. However, I'm fine with this requirement as is but it means there is a little more effort needed to handle reserved frame types across streams.

@lnicco
Copy link
Contributor

lnicco commented May 12, 2019

In our implementation we are interpreting this in the same way: any frame on the control stream received before SETTINGS is a connection error. Even reserved frame types.

In general the way we handle this is by discarding frame types that are forbidden rather than checking frame types that are allowed on each stream.

Here is a snippet from our code.

ParseResult HQControlCodec::checkFrameAllowed(FrameType type) {
  switch (type) {
    case hq::FrameType::DATA:
    case hq::FrameType::HEADERS:
    case hq::FrameType::PUSH_PROMISE:
      return HTTP3::ErrorCode::HTTP_WRONG_STREAM;
    default:
      break;
  }

  // SETTINGS MUST be the first frame on an HQ Control Stream
  if (!receivedSettings_ && type != hq::FrameType::SETTINGS) {
    return HTTP3::ErrorCode::HTTP_MISSING_SETTINGS;
  }
  // .. and a bunch of other checks here ..
  // .. 
}

In this way we avoid the extra effort needed to handle reserved frame types that you mentioned, if I am reading your statement correctly.
Basically, some frame types are explicitly disallowed. All other frame types are equal, reserved or non-reserved. Do you foresee an extension possibly benefiting from sending a frame before settings ?

@LPardue LPardue changed the title http3: Are reserved frames allowed before SETTINGS? http3: Are non-core frames allowed before SETTINGS? May 12, 2019
@LPardue LPardue changed the title http3: Are non-core frames allowed before SETTINGS? http3: Are reserved and non-core frames allowed before SETTINGS? May 12, 2019
@LPardue
Copy link
Member Author

LPardue commented May 12, 2019

Thanks @lnicco you made me realise I asked the question wrong.

This is about reserved and non-core frames. Section 7 says:

Implementations MUST ignore unknown or unsupported values in all
extensible protocol elements. Implementations MUST discard frames ...

I have no strong use case for delivering frames before SETTINGS. But I worry it falls into a slight grey area that might affect interop. I think the correct answer is that the control stream is special case and we should clarify that in Section 7.

@kazuho
Copy link
Member

kazuho commented May 12, 2019

My preference goes to retaining the requirement as-is. This is the way H2 has worked, and I am not sure if there is a practical reason to change.

FWIW, we have a similar requirement for PRIORITY frame too, which I'm also fine with: A PRIORITY frame received after other frames on a request stream MUST be treated as a connection error of type HTTP_UNEXPECTED_FRAME (section 4.2.3).

@LPardue
Copy link
Member Author

LPardue commented May 12, 2019

Ooh yeah, that's a good catch too.

Table 1 in section 4 seems to actually highlight this behaviour with the (1) notation. And we sort of have some text that captures this in section 4:

Certain frames can only occur as the first frame of a particular stream type; these are indicated in Table 1 with a (1). Specific guidance is provided in the relevant section.

We could add tweaks or further guidance to this paragraph ( in a PR that also fixes #2692) that makes things explicitly clear .

@MikeBishop MikeBishop added -http editorial An issue that does not affect the design of the protocol; does not require consensus. labels May 13, 2019
@MikeBishop
Copy link
Contributor

On receipt, reserved is irrelevant -- it's just an unknown frame type. The reservation is simply to permit senders to generate unknown frame types without accidental collision to a real extension the client might support. So the question is whether "ignoring" unknown frame types means pretending that they weren't even there, and therefore the subsequent frame might still be the first frame on the stream.

I'm inclined to say no -- you take no action on a frame you don't recognize, but that doesn't mean that the next frame was the first frame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants