Subject: [quicwg/base-drafts] Rework flow of Push ID (#3925)

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jul 2020 08:23:02 -0700

The Push ID text was divided up between the definitions of various frames and sprinkled into the description of Server Push.  This rearranges a bit to give a &quot;lifecycle&quot; of a Push ID.  I&#39;d really like to have a separate subsection for Push IDs, but they&#39;re fundamental enough to knowing how Server Push works in HTTP/3 that I don&#39;t feel like I can do that without moving most of the section to more subsections.

No normative changes here, just some moved text and a little bit of new prose.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Rework flow of Push ID

-- File Changes --

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

-- Patch Links --

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

---

From: Bence Béky <[email-masked]>
Date: Tue, 21 Jul 2020 10:38:28 -0700

@bencebeky requested changes on this pull request.

LGTM with minor nits

> -same order, and both the name and the value in each field MUST be exact
-matches.
-
-Server push is only enabled on a connection when a client sends a MAX_PUSH_ID
-frame; see {{frame-max-push-id}}. A server cannot use server push until it
-receives a MAX_PUSH_ID frame. A client sends additional MAX_PUSH_ID frames to
-control the number of pushes that a server can promise. A server SHOULD use Push
-IDs sequentially, starting at 0. A client MUST treat receipt of a push stream
-with a Push ID that is greater than the maximum Push ID as a connection error of
-type H3_ID_ERROR.
+Each server push is identified by a unique Push ID, which is used to refer to
+the push in various contexts throughout the lifetime of the connection.
+
+The Push ID space begins at zero, and ends at a maximum value set by the
+MAX_PUSH_ID frame; see {{frame-max-push-id}}.  Server push is only enabled on a
+connection when a client sends a MAX_PUSH_ID frame.  A client sends additional

"Server push is only enabled on a connection when a client sends a MAX_PUSH_ID frame."  I know this sentence is identical to what it was before, but I have minor issues with it.  First, after "a connection" is introduced, I believe it should be "the client" instead of "a client", because "a connection" only has one client.  Second, "after" would be more appropriate than "when", because server push is enabled not only at the moment of the client sending the frame, but for the rest of the connection.

> -
-Server push is only enabled on a connection when a client sends a MAX_PUSH_ID
-frame; see {{frame-max-push-id}}. A server cannot use server push until it
-receives a MAX_PUSH_ID frame. A client sends additional MAX_PUSH_ID frames to
-control the number of pushes that a server can promise. A server SHOULD use Push
-IDs sequentially, starting at 0. A client MUST treat receipt of a push stream
-with a Push ID that is greater than the maximum Push ID as a connection error of
-type H3_ID_ERROR.
+Each server push is identified by a unique Push ID, which is used to refer to
+the push in various contexts throughout the lifetime of the connection.
+
+The Push ID space begins at zero, and ends at a maximum value set by the
+MAX_PUSH_ID frame; see {{frame-max-push-id}}.  Server push is only enabled on a
+connection when a client sends a MAX_PUSH_ID frame.  A client sends additional
+MAX_PUSH_ID frames to control the number of pushes that a server can promise.  A
+server SHOULD use Push IDs sequentially.  A client MUST treat receipt of a push

It might be beneficial to keep the "starting with 0" phrase at the end of the sentence, because the first sentence of this paragraph does not explicitly prescribe that a server SHOULD start with zero.

> -Each server push is identified by a unique Push ID. This Push ID is used in one
-or more PUSH_PROMISE frames (see {{frame-push-promise}}) that carry the request
-fields, then included with the push stream which ultimately fulfills those
-promises. When the same Push ID is promised on multiple request streams, the
-decompressed request field sections MUST contain the same fields in the
-same order, and both the name and the value in each field MUST be exact
-matches.
-
-Server push is only enabled on a connection when a client sends a MAX_PUSH_ID
-frame; see {{frame-max-push-id}}. A server cannot use server push until it
-receives a MAX_PUSH_ID frame. A client sends additional MAX_PUSH_ID frames to
-control the number of pushes that a server can promise. A server SHOULD use Push
-IDs sequentially, starting at 0. A client MUST treat receipt of a push stream
-with a Push ID that is greater than the maximum Push ID as a connection error of
-type H3_ID_ERROR.
+Each server push is identified by a unique Push ID, which is used to refer to

It might be beneficial to state that it is the server that assigns Push IDs to pushes (even though it is not only that server that can use Push IDs to refer to pushes).

-- 
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/3925#pullrequestreview-452672403

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 21 Jul 2020 11:34:54 -0700

@LPardue commented on this pull request.

I think both old and new are correct: which means there was a potential bug in the spec. Reasoning: my old reading was `server SHOULD start at 0 && use sequentially` but the new text seems slightly different. If the client and server disagree about how the ID space is used then there is the potential for interoperability problems. If a client thinks the first push ID is 0, then it can make assumptions about its accounting. If a server wants to send the first push ID as non-zero then there is no signal for it to ask for a credit window shift - it's just blocked. This seems to invalidate the statement that push is only enabled when MAX_PUSH_ID is sent. 

I'm wondering if MUST start at 0 makes sense here, even if that means a separate design issue.

-- 
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/3925#discussion_r458307067

---

From: Bence Béky <[email-masked]>
Date: Tue, 21 Jul 2020 13:35:31 -0700

@bencebeky commented on this pull request.

I agree that this would be an interoperability issue.  However, I think SHOULD is just fine here.  In my mind MUST is something that the peer should be able to enforce, and since streams can arrive out of order, in this case the client cannot detect if the server starts push IDs from a positive integer or if the promise with push ID 0 is delayed.

I think the statement that push is only enabled when MAX_PUSH_ID is sent is still valid.  If no MAX_PUSH_ID is sent, push is not enabled.  If a MAX_PUSH_ID is sent by the client with any value, server push is technically enabled, even if a server that insists on starting push IDs from a larger integer would be not willing to push.  Am I misunderstanding your statement?

That being said, this sentence might flow better if rephrased as "In particular, a server is not allowed to push until it receives a MAX_PUSH_ID frame.".

-- 
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/3925#discussion_r458371555

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 21 Jul 2020 13:53:41 -0700

Good point about detecting difficulties 

-- 
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/3925#discussion_r458380946

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 21 Jul 2020 15:21:01 -0700

> +Push stream data can also arrive after a client has canceled a push. In this
+case, the client can abort reading the stream with an error code of

Did we lose the `SHOULD send a CANCEL_PUSH`? Does that matter?

-- 
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/3925#pullrequestreview-452870214

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jul 2020 15:29:47 -0700

@MikeBishop pushed 1 commit.

836b5bbcef80d6aae5215f4d5bab14c0faf0d459  Feedback from Bence/Lucas

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3925/files/0764b9d94f41d2ae3b96401db0f2477ee4f7fa52..836b5bbcef80d6aae5215f4d5bab14c0faf0d459

---

From: Mike Bishop <[email-masked]>
Date: Tue, 21 Jul 2020 15:31:59 -0700

@MikeBishop commented on this pull request.

It moved up to 827, though I demoted it from a SHOULD to a statement about how the frame is used.  If someone cares about the difference, I can re-RECOMMEND 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/3925#discussion_r458426785

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 21 Jul 2020 15:37:15 -0700

I'm indifferent 

-- 
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/3925#discussion_r458428792

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 21 Jul 2020 15:37:18 -0700

@LPardue 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/3925#pullrequestreview-452877483

---

From: Bence Béky <[email-masked]>
Date: Wed, 22 Jul 2020 05:16:35 -0700

Is this change still editorial if normative language is changed?  I don't feel strongly otherwise: I think the SHOULD was justified, but the current test flows better without it given that the following sentence does not have SHOULD 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/3925#discussion_r458747540

---

From: Bence Béky <[email-masked]>
Date: Wed, 22 Jul 2020 05:16:46 -0700

@bencebeky 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/3925#pullrequestreview-453259152

---

From: Martin Thomson <[email-masked]>
Date: Wed, 22 Jul 2020 05:20:49 -0700

@martinthomson approved this pull request.

+The Push ID space begins at zero, and ends at a maximum value set by the
+MAX_PUSH_ID frame; see {{frame-max-push-id}}.  In particular, a server is not
+able to push until after the client sends a MAX_PUSH_ID frame.  A client sends
+additional MAX_PUSH_ID frames to control the number of pushes that a server can
+promise.  A server SHOULD use Push IDs sequentially, beginning from zero.  A
+client MUST treat receipt of a push stream with a Push ID that is greater than
+the maximum Push ID as a connection error of type H3_ID_ERROR.
+
+The Push ID is used in one or more PUSH_PROMISE frames ({{frame-push-promise}})
+that carry the header section of the request message.  These frames are sent on
+the request stream which generated the push.  This allows the server push to be
+associated with a client request.  When the same Push ID is promised on multiple
+request streams, the decompressed request field sections MUST contain the same
+fields in the same order, and both the name and the value in each field MUST be
+exact matches.

"matches" sometimes means something other than equality.

```suggestion
identical.
```

> +MAX_PUSH_ID frame; see {{frame-max-push-id}}.  In particular, a server is not
+able to push until after the client sends a MAX_PUSH_ID frame.  A client sends
+additional MAX_PUSH_ID frames to control the number of pushes that a server can
+promise.  A server SHOULD use Push IDs sequentially, beginning from zero.  A
+client MUST treat receipt of a push stream with a Push ID that is greater than
+the maximum Push ID as a connection error of type H3_ID_ERROR.
+
+The Push ID is used in one or more PUSH_PROMISE frames ({{frame-push-promise}})
+that carry the header section of the request message.  These frames are sent on
+the request stream which generated the push.  This allows the server push to be
+associated with a client request.  When the same Push ID is promised on multiple
+request streams, the decompressed request field sections MUST contain the same
+fields in the same order, and both the name and the value in each field MUST be
+exact matches.
+
+The Push ID is then included with the push stream which ultimately fulfills

```suggestion
The Push ID is then included with the push stream that ultimately fulfills
```

> @@ -907,9 +910,9 @@ graceful shutdown of a connection by sending a GOAWAY frame ({{frame-goaway}}).
 The GOAWAY frame contains an identifier that indicates to the receiver the range
 of requests or pushes that were or might be processed in this connection.  The
 server sends a client-initiated bidirectional Stream ID; the client sends a Push
-ID.  Requests or pushes with the indicated identifier or greater are rejected by
-the sender of the GOAWAY.  This identifier MAY be zero if no requests or pushes
-were processed.
+ID ({{server-push}}).  Requests or pushes with the indicated identifier or

```suggestion
ID; see {{server-push}}.  Requests or pushes with the indicated identifier or
```

-- 
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/3925#pullrequestreview-453260250

---

From: Bence Béky <[email-masked]>
Date: Wed, 22 Jul 2020 06:13:25 -0700

"same fields" and "both the name and the value in each field MUST be identical" are redundant.  How about "MUST contain identical fields in the same order" or "MUST contain fields with identical name and value in the same order"?

-- 
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/3925#discussion_r458780531

---

From: Mike Bishop <[email-masked]>
Date: Wed, 22 Jul 2020 08:58:53 -0700

b85e25d91014f89481b1ea9fa4593c74b0787f7a  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/3925/files/836b5bbcef80d6aae5215f4d5bab14c0faf0d459..b85e25d91014f89481b1ea9fa4593c74b0787f7a

---

From: Mike Bishop <[email-masked]>
Date: Wed, 22 Jul 2020 08:59:11 -0700

That reference informs the specific term, not the whole statement; I think the parentheses are more appropriate in this instance.

It has to be design if it prohibits something previously permitted or permits something previously prohibited.  A changed recommendation is hazier, and we've sometimes chosen to treat those as design for better visibility to make sure the WG is fully on-board, but they don't have to 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/3925#discussion_r458900246

---

From: Mike Bishop <[email-masked]>
Date: Wed, 22 Jul 2020 09:01:12 -0700

5542b522de2c5c19e56b9a7d0a347971388d0fce  Update draft-ietf-quic-http.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3925/files/b85e25d91014f89481b1ea9fa4593c74b0787f7a..5542b522de2c5c19e56b9a7d0a347971388d0fce

---

From: Bence Béky <[email-masked]>
Date: Wed, 22 Jul 2020 10:45:11 -0700

Okay, that makes sense.  I'm fine with this change then.  Thanks for the explanation.

-- 
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/3925#discussion_r458971497

---

From: Bence Béky <[email-masked]>
Date: Wed, 22 Jul 2020 11:49:49 -0700

+Each server push is assigned a unique Push ID by the server which is used to
+refer to the push in various contexts throughout the lifetime of the connection.

"A client sends additional MAX_PUSH_ID frames...".  This sentence kind of implies that the very first MAX_PUSH_ID frame does not control the number of pushed that a server is allowed to promise, only the subsequent MAX_PUSH_ID frames.  Maybe remove this sentence altogether, because {{frame-max-push-id}} is already doing a great job explaining how the client can send such frames to allow for new push promises as the server fulfills or cancels existing ones?

-- 
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/3925#pullrequestreview-453592674

---

From: Mike Bishop <[email-masked]>
Date: Wed, 22 Jul 2020 13:23:26 -0700

Actually, it turns out the same SHOULD is present in the CANCEL_PUSH section anyway:

> When a server sends CANCEL_PUSH, it is indicating that it will not be fulfilling
> a promise.  The client cannot expect the corresponding promise to be fulfilled,
> unless it has already received and processed the promised response. A server
> SHOULD send a CANCEL_PUSH even if it has opened the corresponding 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/3925#discussion_r459060561

---

From: Mike Bishop <[email-masked]>
Date: Tue, 28 Jul 2020 18:47:54 +0000 (UTC)

a248e98f13246c11e6387f62f63c815ce53297e1  Not additional

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3925/files/5542b522de2c5c19e56b9a7d0a347971388d0fce..a248e98f13246c11e6387f62f63c815ce53297e1

---

From: Bence Béky <[email-masked]>
Date: Wed, 29 Jul 2020 07:23:34 -0700

Thank you, https://github.com/quicwg/base-drafts/commit/a248e98f13246c11e6387f62f63c815ce53297e1 solves this nicely.

-- 
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/3925#discussion_r462338714

---

From: Bence Béky <[email-masked]>
Date: Wed, 29 Jul 2020 07:25:38 -0700

-- 
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/3925#pullrequestreview-457575592