Subject: [quicwg/base-drafts] A tiny nit: the ClientHello is not subject to MAX_STREAM_DATA. (#803)

From: ekr <[email-masked]>
Date: Thu, 28 Sep 2017 07:25:13 +0000 (UTC)

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * A tiny nit: the ClientHello is not subject to MAX_STREAM_DATA.

-- File Changes --

    M draft-ietf-quic-transport.md (14)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/803.patch
https://github.com/quicwg/base-drafts/pull/803.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/803

---

From: janaiyengar <[email-masked]>
Date: Thu, 28 Sep 2017 21:04:21 +0000 (UTC)

janaiyengar commented on this pull request.

Thanks, ekr, LGTM, but a couple of comments.

> @@ -2759,10 +2759,16 @@ ordered byte-stream.  Data received out of order MUST be buffered for later
 delivery, as long as it is not in violation of the receiver's flow control
 limits.
 
-An endpoint MUST NOT send data on any stream without ensuring that it 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.  Stream
-0 is still subject to stream-level data limits and MAX_STREAM_DATA.
+An endpoint MUST NOT send data on any stream without ensuring that it
+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
+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

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")?

> @@ -2759,10 +2759,16 @@ ordered byte-stream.  Data received out of order MUST be buffered for later
 delivery, as long as it is not in violation of the receiver's flow control
 limits.
 
-An endpoint MUST NOT send data on any stream without ensuring that it 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.  Stream
-0 is still subject to stream-level data limits and MAX_STREAM_DATA.
+An endpoint MUST NOT send data on any stream without ensuring that it
+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
+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

nit: "would not permit" --> "does not permit"

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/803#pullrequestreview-65998044

---

From: ekr <[email-masked]>
Date: Thu, 28 Sep 2017 14:19:24 -0700

@ekr pushed 1 commit.

617dc33  Nits from Jana

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/803/files/fc31b7ea51df8a94de6a9b019a6b9ce7f86f3faa..617dc338b317d217441aeac48756bd4a1bc11925

---

From: ekr <[email-masked]>
Date: Thu, 28 Sep 2017 21:19:26 +0000 (UTC)

ekr commented on this pull request.

Done.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/803#discussion_r141739147

---

From: ekr <[email-masked]>
Date: Thu, 28 Sep 2017 14:19:56 -0700

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.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/803#discussion_r141739248

---

From: janaiyengar <[email-masked]>
Date: Thu, 28 Sep 2017 21:33:11 +0000 (UTC)

Ah, right, good point. Your wording seems better, so go with it.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/803#discussion_r141741985

---

From: janaiyengar <[email-masked]>
Date: Thu, 28 Sep 2017 14:35:19 -0700

editorial nit: Can you say "initial cryptographic message from the client" instead of TLS ClientHello, for consistency (here and below)?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/803#pullrequestreview-66006829

---

From: ekr <[email-masked]>
Date: Fri, 29 Sep 2017 04:59:55 +0000 (UTC)

@ekr pushed 1 commit.

8cf389d  Jana's comments plus bonus editorial fix

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/803/files/617dc338b317d217441aeac48756bd4a1bc11925..8cf389da6e5ccc118abcfb8d714ee101c752fdb0