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

Minor inconsistency of error when MAX_STREAMS exceeds 2^60 #3781

Closed
LPardue opened this issue Jun 21, 2020 · 4 comments · Fixed by #4002
Closed

Minor inconsistency of error when MAX_STREAMS exceeds 2^60 #3781

LPardue opened this issue Jun 21, 2020 · 4 comments · Fixed by #4002
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@LPardue
Copy link
Member

LPardue commented Jun 21, 2020

Over on cloudflare/quiche#565 we had a discussion about whether a MAX_STREAMS frame with value exceeding 2^60 generates a FRAME_ENCODING or a STREAM_LIMIT error.

Draft 29 Section 19.11 says:

Maximum Streams: A count of the cumulative number of streams of the
corresponding type that can be opened over the lifetime of the
connection. This value cannot exceed 2^60, as it is not possible
to encode stream IDs larger than 2^62-1. Receipt of a frame that
permits opening of a stream larger than this limit MUST be treated
as a FRAME_ENCODING_ERROR.

but as @goelvidhi points out, Section 4.5 says

If a max_streams transport parameter or MAX_STREAMS frame is received
with a value greater than 2^60, this would allow a maximum stream ID
that cannot be expressed as a variable-length integer; see
Section 16. If either is received, the connection MUST be closed
immediately with a connection error of type STREAM_LIMIT_ERROR

Meanwhile, for the STREAMS_BLOCKED frame in Section 19.15 we have:

Receipt of a frame that encodes a larger stream ID MUST be treated as a STREAM_LIMIT_ERROR or a FRAME_ENCODING_ERROR.

I suspect both Vidhi and I are correct, bogus MAX_STREAMS can be treated as either STREAM_LIMIT or FRAME_ENCODING. If so, its probably a minor editorial fix to state either/or in both the sections.

@larseggert larseggert added this to Triage in Late Stage Processing via automation Jun 22, 2020
@martinthomson martinthomson added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Jun 26, 2020
martinthomson added a commit that referenced this issue Jun 26, 2020
STREAM_LIMIT_ERROR is fine.
FRAME_ENCODING_ERROR is fine.

We just need to be consistent in mentioning both throughout.

Closes #3781.
@project-bot project-bot bot moved this from Triage to Editorial Issues in Late Stage Processing Jun 26, 2020
@ianswett
Copy link
Contributor

@marten-seemann suggested that it should only be a FRAME_ENCODING_ERROR, as it's invalid to send a MAX_STREAMS larger than 2^60 under any circumstance, regardless of connection state, and I agree that's the clearest application of our principles.

@kazuho
Copy link
Member

kazuho commented Jul 13, 2020

I agree that requiring the use of FRAME_ENOCDING_ERROR is fine.

Even though we occasionally allow an endpoint to use multiple error codes when there is more than one way of checking the status (i.e. FLOW_CONTROL_ERROR vs. FRAME_ENCODING_ERROR for STREAM frames), the check required in this particular case is purely unrelated to the connection state. Hence my +1.

@LPardue
Copy link
Member Author

LPardue commented Jul 14, 2020

I would be happy with a single error too. I'm not fussy about the actual code used as long as the spec is clear and consistent. That means that we'd need to reduce STREAMS_BLOCKED to one error code too, and potentially any other situation that allows either.

@martinthomson
Copy link
Member

I'm going to drop #3792. I agree with the comments suggesting that FRAME_ENCODING_ERROR is the right answer.

Note that you might still receive a different error, as we permit endpoints that detect multiple errors to send any that it detects, and this case clearly justifies use of STREAM_LIMIT_ERROR.

martinthomson added a commit that referenced this issue Aug 17, 2020
This is an error that can be detected in frame parsing without
additional context.

Closes #3781.
Late Stage Processing automation moved this from Editorial Issues to Issue Handled Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
Late Stage Processing
  
Issue Handled
5 participants