-
Notifications
You must be signed in to change notification settings - Fork 203
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
A tiny nit: the ClientHello is not subject to MAX_STREAM_DATA. #803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, ekr, LGTM, but a couple of comments.
draft-ietf-quic-transport.md
Outdated
limits established by MAX_DATA. Data on stream 0 other than the | ||
TLS ClientHello is still subject to stream-level data limits and | ||
MAX_STREAM_DATA. The TLS ClientHello is exempt from flow control because it needs | ||
to be sent in one piece regardless of the server's flow control |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this phrase: "it needs to be sent in one piece". Do you mean that it needs to fit within a single packet (because we do require that, in Section 7.2.) If so, can you use the same language as in 7.2 (" initial cryptographic handshake message MUST be sent in a single packet")?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
draft-ietf-quic-transport.md
Outdated
MAX_STREAM_DATA. The TLS ClientHello is exempt from flow control because it needs | ||
to be sent in one piece regardless of the server's flow control | ||
state. This rule applies even for 0-RTT handshakes where the | ||
remembered value of MAX_STREAM_DATA would not permit sending |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "would not permit" --> "does not permit"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the change, but I actually think this is wrong because this is a counterfactual. I.e., it wouldn't permit it if taken literally, but we do not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, good point. Your wording seems better, so go with it.
draft-ietf-quic-transport.md
Outdated
is within the data limits set by its peer. The cryptographic | ||
handshake stream, Stream 0, is exempt from the connection-level data | ||
limits established by MAX_DATA. Data on stream 0 other than the | ||
TLS ClientHello is still subject to stream-level data limits and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
editorial nit: Can you say "initial cryptographic message from the client" instead of TLS ClientHello, for consistency (here and below)?
No description provided.