Subject: [quicwg/base-drafts] STOP_SENDING opens streams (#2013)

From: Mike Bishop <[email-masked]>
Date: Thu, 15 Nov 2018 14:47:51 -0800

Fixes #1797, hopefully for real this time.

This is working with the existing text, but frankly, it feels a little wonky to say that receiving a frame for the sending part opens the receiving part, which opens the sending part, so the frame on the sending part is now valid to have received.

I'm not certain there's a better way to frame that, but we might think about it.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * STOP_SENDING opens streams

-- File Changes --

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

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Thu, 15 Nov 2018 14:52:40 -0800

mikkelfj commented on this pull request.

> @@ -4276,12 +4280,12 @@ An endpoint uses a STOP_SENDING frame (type=0x05) to communicate that incoming
 data is being discarded on receipt at application request.  This signals a peer
 to abruptly terminate transmission on a stream.
 
-Receipt of a STOP_SENDING frame is only valid for a send stream that exists and
-is not in the "Ready" state (see {{stream-send-states}}).  Receiving a
-STOP_SENDING frame for a send stream that is "Ready" or non-existent MUST be
-treated as a connection error of type PROTOCOL_VIOLATION.  An endpoint that
-receives a STOP_SENDING frame for a receive-only stream MUST terminate the
-connection with error PROTOCOL_VIOLATION.
+Receipt of a STOP_SENDING frame is invalid for a locally-initiated stream which
+does not exist or is in the "Ready" state (see {{stream-send-states}}).
+Receiving a STOP_SENDING frame for a locally-initiated send stream that is
+"Ready" or non-existent MUST be treated as a connection error of type
+PROTOCOL_VIOLATION.  An endpoint that receives a STOP_SENDING frame for a
+receive-only stream MUST terminate the connection with error PROTOCOL_VIOLATION.

I don't think this handles STOP_SENDING sent on an implicitly opened stream.

-- 
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/2013#pullrequestreview-175594119

---

From: Ryan Hamilton <[email-masked]>
Date: Thu, 15 Nov 2018 17:44:21 -0800

RyanAtGoogle commented on this pull request.

This looks great to me! I agree with your unease about "receiving a frame for the sending part opens the receiving part, which opens the sending part, so the frame on the sending part is now valid to have received." But as you say, working with the existing text, that does seem fine. In any case, this feels like a definitely improvement to the status quo.

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/2013#pullrequestreview-175630826

---

From: ianswett <[email-masked]>
Date: Fri, 16 Nov 2018 08:09:58 -0800

ianswett approved this pull request.

I think this works, but it is a bit awkward.  I don't have any concrete suggestions though.

-- 
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/2013#pullrequestreview-175870714

---

From: Subodh Iyengar <[email-masked]>
Date: Fri, 16 Nov 2018 09:26:28 -0800

+1 on this. We were doing this implicitly in our implementation 

-- 
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/2013#issuecomment-439466118

---

From: Subodh Iyengar <[email-masked]>
Date: Fri, 16 Nov 2018 09:29:50 -0800

siyengar commented on this pull request.

I think senders are required to open streams in order so this should be fine

I think senders are required to open streams in order so this should be fine

-- 
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/2013#discussion_r234287786

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 16 Nov 2018 14:17:48 -0800

+1. We do this too.

-- 
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/2013#issuecomment-439546298

---

From: Marten Seemann <[email-masked]>
Date: Fri, 16 Nov 2018 17:40:04 -0800

marten-seemann approved this pull request.

"does not exist" sounds a bit weird. In my implementation, when a stream is fully closed, I delete it from my map of streams at some point. At that moment, it does not exist any more, but receipt of a (delayed) STOP_SENDING is still a valid thing.

Maybe we can say "has not yet been created".

-- 
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/2013#pullrequestreview-176034256

---

From: Martin Thomson <[email-masked]>
Date: Sun, 18 Nov 2018 16:44:17 -0800

martinthomson approved this pull request.

```suggestion
Receipt of a STOP_SENDING frame is invalid for a locally-initiated stream that
```

```suggestion
has not yet been created or is in the "Ready" state (see {{stream-send-states}}).
```

-- 
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/2013#pullrequestreview-176116687

---

From: Mike Bishop <[email-masked]>
Date: Mon, 19 Nov 2018 11:07:50 -0800

@MikeBishop pushed 1 commit.

c149b56  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2013/files/06b4cbd925ffc9a97e1b3ec7d22e750003232ad6..c149b56a4dae31b7d69956aac7830d79b2dd997d

---

From: Mike Bishop <[email-masked]>
Date: Mon, 19 Nov 2018 11:11:10 -0800

a172c5a  Rewrap from GH suggestions

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Nov 2018 13:39:00 -0800

Merged #2013 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/2013#event-1975816771