Subject: [quicwg/base-drafts] Close the connection if MAX_STREAMS is too large (#2499)

From: ianswett <[email-masked]>
Date: Tue, 05 Mar 2019 14:42:40 -0800

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

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

-- Commit Summary --

  * Close the connection if MAX_STREAMS is too large

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2499.patch
https://github.com/quicwg/base-drafts/pull/2499.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/2499

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 05 Mar 2019 14:52:19 -0800

This LGTM, but it will need chair approval eventually.

-- 
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/2499#issuecomment-469891450

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 05 Mar 2019 14:52:35 -0800

janaiyengar approved this pull request.

Thanks for writing this up!

-- 
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/2499#pullrequestreview-210963660

---

From: Martin Thomson <[email-masked]>
Date: Tue, 05 Mar 2019 14:56:34 -0800

martinthomson approved this pull request.

-- 
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/2499#pullrequestreview-210965099

---

From: Marten Seemann <[email-masked]>
Date: Tue, 05 Mar 2019 16:20:52 -0800

This is also applies to the transport parameters, doesn’t 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/2499#issuecomment-469913776

---

From: ianswett <[email-masked]>
Date: Tue, 05 Mar 2019 16:21:34 -0800

Good point, I think it does.

-- 
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/2499#issuecomment-469913905

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 05 Mar 2019 19:01:01 -0800

kazuho commented on this pull request.

> @@ -849,7 +849,10 @@ initial_stream_id_for_type) can be opened (see {{long-packet-types}}).  Initial
 limits are set in the transport parameters (see
 {{transport-parameter-definitions}}) and subsequently limits are advertised
 using MAX_STREAMS frames ({{frame-max-streams}}). Separate limits apply to
-unidirectional and bidirectional streams.
+unidirectional and bidirectional streams. When a MAX_STREAMS frame is received
+that permits more streams than can be expressed by the stream ID
+(i.e. greater than 2^60 - 1), the connection MUST be immediately closed with a

I wonder if "greater than 2^60 - 1" is correct. 2^60 is a valid number to be carried by a MAX_STREAM frame.

To avoid confusion (between number of streams and the maximum stream IDs), it might be a good idea to simply state something like: _When a MAX_STREAM frame carrying Maximum Streams greater than 2^60 is received, the connection MUST be ..._

-- 
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/2499#pullrequestreview-211024905

---

From: Martin Thomson <[email-masked]>
Date: Tue, 05 Mar 2019 19:33:31 -0800

martinthomson commented on this pull request.

Yeah, I think that is better.  Or "If a MAX_STREAMS carries a value of 2^60 or greater (which implies a maximum stream ID that cannot be expressed in the protocol), then the connection MUST ..."

-- 
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/2499#discussion_r262781169

---

From: Mike Bishop <[email-masked]>
Date: Wed, 06 Mar 2019 22:51:35 +0000 (UTC)

MikeBishop approved this pull request.

I like Martin's wording, but I'm okay with this change.

-- 
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/2499#pullrequestreview-211518845

---

From: Martin Thomson <[email-masked]>
Date: Wed, 06 Mar 2019 14:54:34 -0800

Just a reminder that this shouldn't land without fixing the transport parameter.

-- 
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/2499#pullrequestreview-211519947

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 06 Mar 2019 16:56:40 -0800

@martinthomson's text looks fine with one exception: s/a value of 2^60 or greater/a value greater than 2^60/.

2^60 is a valid number to be carried by MAX_STREAMS.

-- 
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/2499#discussion_r263200434

---

From: Martin Thomson <[email-masked]>
Date: Wed, 06 Mar 2019 16:58:33 -0800

Yep, my bad.  Fence-post error on my part.

-- 
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/2499#discussion_r263200815

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 11:13:55 -0800

@ianswett pushed 1 commit.

b737e89ad4ed987984ece6b296c484eb0f8af06b  Martin/Kazuho's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/4edeb3a5c323b54d37302749fdc2156d6a172332..b737e89ad4ed987984ece6b296c484eb0f8af06b

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 19:15:24 +0000 (UTC)

I now mention the transport parameters and incorporated a version of Martin's suggested text.  PTAL.

-- 
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/2499#issuecomment-470656865

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 11:22:52 -0800

fc64707b237c37fee03e21caabb256d30ace5074  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/b737e89ad4ed987984ece6b296c484eb0f8af06b..fc64707b237c37fee03e21caabb256d30ace5074

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 07 Mar 2019 20:50:37 +0000 (UTC)

kazuho approved this pull request.

-- 
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/2499#pullrequestreview-212012424

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Mar 2019 12:52:50 -0800

couple of nits, but LGTM

> @@ -851,6 +851,11 @@ limits are set in the transport parameters (see
 using MAX_STREAMS frames ({{frame-max-streams}}). Separate limits apply to
 unidirectional and bidirectional streams.
 
+If a max_streams transport parameter or MAX_STREAMS frame carries a value
+greater than 2^60, which implies a maximum stream ID that cannot be expressed,

```suggestion
greater than 2^60, which implies a maximum stream ID that cannot be expressed in later frames (see {{frame-formats}}),
```

> @@ -851,6 +851,11 @@ limits are set in the transport parameters (see
 using MAX_STREAMS frames ({{frame-max-streams}}). Separate limits apply to
 unidirectional and bidirectional streams.
 
+If a max_streams transport parameter or MAX_STREAMS frame carries a value

```suggestion
If a max_streams transport parameter or MAX_STREAMS frame is received with a value
```

-- 
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/2499#pullrequestreview-212013092

---

From: Martin Thomson <[email-masked]>
Date: Thu, 07 Mar 2019 14:19:48 -0800

I don't think that we need to say this.  It's strictly more correct, but the extra words only obscure the meaning.  Isn't it obvious that we're only talking about QUIC?

-- 
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/2499#discussion_r263593745

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 14:20:34 -0800

01d76802c343bea31b1d8428e8558b5722973d86  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/fc64707b237c37fee03e21caabb256d30ace5074..01d76802c343bea31b1d8428e8558b5722973d86

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 14:21:15 -0800

ianswett commented on this pull request.

Agreed.

-- 
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/2499#discussion_r263594191

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 22:21:55 +0000 (UTC)

74c725afef91dc3dfeb659751d3512d07f7558aa  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/01d76802c343bea31b1d8428e8558b5722973d86..74c725afef91dc3dfeb659751d3512d07f7558aa

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 22:24:55 +0000 (UTC)

1ecbbea933dd4f4d382523f6d51e7e145576a5b9  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/74c725afef91dc3dfeb659751d3512d07f7558aa..1ecbbea933dd4f4d382523f6d51e7e145576a5b9

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 07 Mar 2019 15:42:13 -0800

janaiyengar commented on this pull request.

My addition was (perhaps poorly) trying to articulate the difference between expressing it in transport params/MAX_STREAMS vs. in other frames. Saying that it's not expressible isn't accurate -- it was "expressed" when we received it -- what we want to say is that it's not expressible in other frames, specifically as a varint. 

Maybe what I should have suggested is 
"cannot be expressed as a variable-length integer (see {{integer-encoding}})."

-- 
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/2499#discussion_r263616643

---

From: ianswett <[email-masked]>
Date: Thu, 07 Mar 2019 16:05:59 -0800

0a7ac9f173420f081d2bd773d9943d2929cf3ff7  Jana's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/1ecbbea933dd4f4d382523f6d51e7e145576a5b9..0a7ac9f173420f081d2bd773d9943d2929cf3ff7

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Mar 2019 16:58:08 -0800

Wordsmithing, but content is fine.

> @@ -851,6 +851,12 @@ limits are set in the transport parameters (see
 
+If a max_streams transport parameter or MAX_STREAMS frame is received with a
+value greater than 2^60, this allows a maximum stream ID that cannot be
+expressed as a variable-length integer (see {{integer-encoding}}).
+If either is received, the connection MUST be immediately closed with a

"be immediately closed" feels like odd word order, and doesn't occur elsewhere in the document.  Maybe "be closed immediately" or simply "be closed"?

-- 
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/2499#pullrequestreview-212536274

---

From: ianswett <[email-masked]>
Date: Sat, 09 Mar 2019 01:28:33 -0800

8ecbcb2799a9ed27844b476c098328897a334ca3  Mike's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/0a7ac9f173420f081d2bd773d9943d2929cf3ff7..8ecbcb2799a9ed27844b476c098328897a334ca3

---

From: ianswett <[email-masked]>
Date: Sat, 09 Mar 2019 01:28:55 -0800

Changed to closed immediately

-- 
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/2499#discussion_r263992921

---

From: Marten Seemann <[email-masked]>
Date: Sat, 09 Mar 2019 02:09:56 -0800

marten-seemann commented on this pull request.

Maybe better: ... this value would be larger than the maximum stream ID that can be expresed as a variable-length integer.

-- 
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/2499#pullrequestreview-212555592

---

From: ianswett <[email-masked]>
Date: Sat, 09 Mar 2019 06:43:26 -0800

I think I'm ok with the existing text at this point, but if others prefer yours, I'll change 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/2499#discussion_r264001606

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 11 Mar 2019 06:03:56 -0700

2^60 is too large as well, as 2^62 cannot be expressed as a variable-length integer, either.

-- 
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/2499#issuecomment-471529776

---

From: ianswett <[email-masked]>
Date: Mon, 11 Mar 2019 06:56:46 -0700

I believe 2^60 means stream IDs of that type with Stream IDs less than 2^62 can be opened, but those equal and greater cannot.

I MAX_STREAMS of 1 means a single stream can be opened, and that stream will have an ID less than 1 * 2^2 = 4.

-- 
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/2499#issuecomment-471548395

---

From: Dmitri Tikhonov <[email-masked]>
Date: Mon, 11 Mar 2019 07:03:14 -0700

@ianswett, I see.  You're right -- thanks.

-- 
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/2499#issuecomment-471550891

---

From: Tommy Pauly <[email-masked]>
Date: Wed, 13 Mar 2019 08:41:58 -0700

tfpauly commented on this pull request.

If we're bike-shedding the grammar, I'd vote for `... this would allow a maximum stream ID ...`. Using "would" correlates better to the fact that this is indeed a protocol error and therefore isn't actually allowing anything.

-- 
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/2499#discussion_r265194072

---

From: ianswett <[email-masked]>
Date: Wed, 13 Mar 2019 15:48:40 +0000 (UTC)

SG, 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/2499#discussion_r265197570

---

From: ianswett <[email-masked]>
Date: Wed, 13 Mar 2019 15:52:29 +0000 (UTC)

1281676099be89184bd8a85b59600b4142dc3e47  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2499/files/8ecbcb2799a9ed27844b476c098328897a334ca3..1281676099be89184bd8a85b59600b4142dc3e47

---

From: Tommy Pauly <[email-masked]>
Date: Wed, 13 Mar 2019 09:04:39 -0700

tfpauly approved this pull request.

-- 
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/2499#pullrequestreview-214052769

---

From: Martin Thomson <[email-masked]>
Date: Tue, 19 Mar 2019 16:46:19 -0700

Merged #2499 into master.

-- 
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/2499#event-2215326548