Subject: [quicwg/base-drafts] Improve Push text (#1210)

From: Mike Bishop <[email-masked]>
Date: Thu, 15 Mar 2018 14:32:36 +0000 (UTC)

Attempts to resolve #1175.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Improve Push text

-- File Changes --

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

-- Patch Links --

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

---

From: Lucas Pardue <[email-masked]>
Date: Thu, 15 Mar 2018 17:07:50 +0000 (UTC)

LPardue requested changes on this pull request.

Looks good, some minor points:

1) how about a ref to the PUSH_PROMISE frame?
2) the Push ID text is still awkward. In one paragraph it refs to 4.2.6 and in the following paragraph some text explains what Push ID means. I think some editing could fix this.
3) "includes request header fields attributed to the request." I don't understand what this means. Can we just say pseudo-request or something?
4) I noticed the document structure 
causes the CANCEL_PUSH frame to be  defined before the PUSH_PROMISE frame. Could that be reordered or is it that way for a good reason? 

-- 
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/1210#pullrequestreview-104294951

---

From: Mike Bishop <[email-masked]>
Date: Thu, 15 Mar 2018 10:40:04 -0700

On (4), it's because Martin reused the RST_STREAM codepoint for CANCEL_PUSH, and the frames are presented in codepoint order.  Whether that's a "good" reason is debatable, but that's the reason anyway.  I'll take a look at the others.

-- 
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/1210#issuecomment-373462536

---

From: Mike Bishop <[email-masked]>
Date: Thu, 15 Mar 2018 10:48:15 -0700

@MikeBishop pushed 1 commit.

b9875e7  Lucas's feedback

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

---

From: Martin Thomson <[email-masked]>
Date: Fri, 16 Mar 2018 09:31:32 +0000 (UTC)

martinthomson approved this pull request.

This prompted a few thoughts that we might want to look into more.

> @@ -433,28 +433,43 @@ frame.
 
 ## Server Push
 
-HTTP/QUIC supports server push as described in {{!RFC7540}}. During connection
-establishment, the client enables server push by sending a MAX_PUSH_ID frame
-(see {{frame-max-push-id}}).  A server cannot use server push until it receives
-a MAX_PUSH_ID frame.
+HTTP/QUIC supports server push in a similar manner to {{!RFC7540}}, but uses
+different mechanisms. During connection establishment, the client enables server
+push by sending a MAX_PUSH_ID frame (see {{frame-max-push-id}}). A server cannot
+use server push until it receives a MAX_PUSH_ID frame.

Which makes me wonder (probably for the second time) whether adding a setting is valuable or not.  I guess not because MAX_PUSH_ID can be send immediately after SETTINGS, and having two ways to do the same thing isn't great.

 As with server push for HTTP/2, the server initiates a server push by sending a
-PUSH_PROMISE frame that includes request header fields attributed to the
-request. The PUSH_PROMISE frame is sent on the client-initiated, bidirectional
-stream that carried the request that generated the push.  This allows the server
-push to be associated with a request.  Ordering of a PUSH_PROMISE in relation to
+PUSH_PROMISE frame (see {{frame-push-promise}}) that includes request headers
+for the promised request.  Promised requests MUST be cacheable (see
+{{!RFC7231}}, Section 4.2.3), MUST be safe (see {{RFC7231}}, Section 4.2.1), and
+MUST NOT include a request body.

I think that we want to reference the requirements in h2, not repeat them.

-- 
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/1210#pullrequestreview-104498575

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 16 Mar 2018 02:49:07 -0700

LPardue commented on this pull request.

I'd be happy with that

-- 
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/1210#discussion_r175039739

---

From: Lucas Pardue <[email-masked]>
Date: Fri, 16 Mar 2018 02:50:32 -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/1210#pullrequestreview-104505648

---

From: Mike Bishop <[email-masked]>
Date: Sun, 18 Mar 2018 15:34:53 +0000 (UTC)

MikeBishop commented on this pull request.

Ah, I though part of the point of the original issue was that these requirements weren't in the spec, but just pointed off to RFC7540.  I'll replace this with a pointer to the specific section.

-- 
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/1210#discussion_r175292990

---

From: Mike Bishop <[email-masked]>
Date: Sun, 18 Mar 2018 08:38:53 -0700

## Server Push

By that logic, we shouldn't have transport parameters for max stream ID either -- just send MAX_STREAM_ID frames in your first packet.  But that would prevent servers from using 0.5-RTT data, since they haven't received authenticated packets from the client yet.

HTTP has no such restriction, since all data is inside the crypto.  We can just use the frames.

-- 
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/1210#discussion_r175293110

---

From: Mike Bishop <[email-masked]>
Date: Sun, 18 Mar 2018 16:12:28 +0000 (UTC)

f392bbb  Just reference 7540

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

---

From: Mike Bishop <[email-masked]>
Date: Sun, 18 Mar 2018 09:16:29 -0700

Merged #1210.

-- 
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/1210#event-1527296968