Subject: [quicwg/base-drafts] Servers should always send CANCEL_PUSH (#3700)

From: Mike Bishop <[email-masked]>
Date: Tue, 26 May 2020 15:45:56 -0700

Fixes #3698 by reversing the polarity of a SHOULD.  Doesn&#39;t break interop, but it&#39;s always safer to send CANCEL_PUSH than to omit it, so let&#39;s not recommend omitting it under any circumstances.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Servers should always send CANCEL_PUSH

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 26 May 2020 16:05:27 -0700

@ianswett 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/3700#pullrequestreview-418737059

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 16:20:34 -0700

@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/3700#pullrequestreview-418742697

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 26 May 2020 18:06:32 -0700

@LPardue commented on this pull request.

>  When a server sends CANCEL_PUSH, it is indicating that it will not be fulfilling
-a promise and has not created a push stream.  The client should not expect the
-corresponding promise to be fulfilled.
+a promise.  The client cannot expect the corresponding promise to be fulfilled.
+A server SHOULD send a CANCEL_PUSH even if it has opened the corresponding
+stream.

Maybe not a direct result of this change but now I'm confused what happens if the server sends a CANCEL_PUSH that references a Push ID of a valid completed push stream. Do nothing seems like the right call.

-- 
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/3700#pullrequestreview-418784751

---

From: Dragana Damjanovic <[email-masked]>
Date: Wed, 27 May 2020 05:03:00 -0700

@ddragana commented on this pull request.

I think that is the right call.

-- 
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/3700#discussion_r431063892

---

From: ianswett <[email-masked]>
Date: Wed, 27 May 2020 05:07:38 -0700

@ianswett commented on this pull request.

That matches what would occur in transport, so it does seem like the right call.

-- 
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/3700#discussion_r431066300

---

From: Mike Bishop <[email-masked]>
Date: Wed, 27 May 2020 06:47:12 -0700

@MikeBishop pushed 1 commit.

745efc5d984a479629d973e0c062837c5cdd3cc7  Fulfilled before cancel

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3700/files/618010e47dc5045b154e7bd62577e1e0a59dd178..745efc5d984a479629d973e0c062837c5cdd3cc7

---

From: Mike Bishop <[email-masked]>
Date: Wed, 27 May 2020 06:49:17 -0700

@MikeBishop commented on this pull request.

Added text to this effect.  And that in fact could happen -- if we're suggesting a CANCEL_PUSH whenever the server abandons a stream (i.e. RESET_STREAM), it could be doing that to abort retransmissions even after writing the full payload.  The full payload might still have arrived if there's no loss.

-- 
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/3700#discussion_r431145538

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 27 May 2020 06:54:24 -0700

@LPardue approved this pull request.

lgtm

-- 
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/3700#pullrequestreview-419231741

---

From: Mike Bishop <[email-masked]>
Date: Wed, 27 May 2020 08:27:26 -0700

8934b25be9a655b73da54bdcfe411cb03b0ea71a  A suggested text about a push stream after a CANCEL_PUSH

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3700/files/745efc5d984a479629d973e0c062837c5cdd3cc7..8934b25be9a655b73da54bdcfe411cb03b0ea71a

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 May 2020 18:16:14 -0700

> @@ -1292,9 +1292,10 @@ SHOULD send a CANCEL_PUSH even if it has opened the corresponding stream.
 
 Sending CANCEL_PUSH has no direct effect on the state of existing push streams.
 A client SHOULD NOT send a CANCEL_PUSH when it has already received a
-corresponding push stream.  If a push stream arrives after a client has sent
-CANCEL_PUSH, this MAY be treated as a stream error of type
-H3_STREAM_CREATION_ERROR.
+corresponding push stream.  A push stream may arrive after a client has sent

```suggestion
corresponding push stream.  A push stream could arrive after a client has sent
```

> @@ -1292,9 +1292,10 @@ SHOULD send a CANCEL_PUSH even if it has opened the corresponding stream.
 
 Sending CANCEL_PUSH has no direct effect on the state of existing push streams.
 A client SHOULD NOT send a CANCEL_PUSH when it has already received a
-corresponding push stream.  If a push stream arrives after a client has sent
-CANCEL_PUSH, this MAY be treated as a stream error of type
-H3_STREAM_CREATION_ERROR.
+corresponding push stream.  A push stream may arrive after a client has sent
+CANCEL_PUSH, because a server may have not yet processed the CANCEL_PUSH. The

```suggestion
CANCEL_PUSH, because a server might not have processed the CANCEL_PUSH. The
```

-- 
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/3700#pullrequestreview-419713612

---

From: Mike Bishop <[email-masked]>
Date: Thu, 28 May 2020 06:32:27 -0700

74a113b1dce55fe610ac2cc468d0a60a05224d46  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/3700/files/8934b25be9a655b73da54bdcfe411cb03b0ea71a..74a113b1dce55fe610ac2cc468d0a60a05224d46