Subject: [quicwg/base-drafts] Push ID (#701)

From: Martin Thomson <[email-masked]>
Date: Thu, 27 Jul 2017 06:23:42 +0000 (UTC)

As requested, I split this out of the unidirectional PR (#643).

There is a change here, that needs to be highlighted.  @kazuho points out that without inter-stream ordering guarantees, requests might be triggered on multiple streams.  The server then has an awkward choice: either push multiple times, or risk the client triggering requests when streams arrive out of order.  This contains a fix for that bug.

The fix is to allow the same push ID to be used in multiple PUSH_PROMISE frames.  Then, fulfillment happens at most once.

I think that this is a neat fix, but it creates two more problems:

1. How does a client know when the server will stop referencing a push ID in PUSH_PROMISE?  With reordering, the pushed response might arrive and be consumed before a PUSH_PROMISE that references it.  I think that it might be possible to add a counter to the push stream header that says how many times the server promised this response.  The problem with that is that if a stream that carries the PUSH_PROMISE is reset, then that counter will be too high.  I've decided to just explain the issue and add some weasel wording rather than provide a direct fix.

2. What if the server provides different headers for the same Push ID?  I've chosen to fix this by simply forbidding variations in the content of the header block.  I considered only allowing the headers to appear once, but then you have to deal with that stream being reset.  Repetition should be rare enough, and header compression might be able to help with the extra bytes.

I think that those problems are both less serious than the bug this fixes.  Besides, the first is really only an generalized form of a problem we have already: the stream carrying a PUSH_PROMISE can be reset.  What currently happens is that the server more or less has to send a push stream anyway, even though it can't know whether the client got the PUSH_PROMISE.  With this, a server can use CANCEL_REQUEST.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Define a Push ID

-- File Changes --

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

-- Patch Links --

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

---

From: Mike Bishop <[email-masked]>
Date: Thu, 27 Jul 2017 10:35:19 -0700

I'll note that, with unidirectional streams, we could use a stream from the server for the request headers as well, and then only have them once.  The problem with that is that, until they're processed, the client doesn't know what request it is that it shouldn't be making as it processes the original resource.

-- 
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/701#issuecomment-318432588

---

From: Patrick McManus <[email-masked]>
Date: Thu, 27 Jul 2017 19:42:37 +0000 (UTC)

mcmanus commented on this pull request.

> +A PRIORITY frame that does not reference a request MUST be treated as a
+HTTP_MALFORMED_PRIORITY error, unless it references stream ID 0.  A PRIORITY
+that sets a PUSH_PRIORITIZED or PUSH_DEPENDENT flag, but then references a
+non-existent Push ID MUST be treated as a HTTP_MALFORMED_PRIORITY error.
+
+The length of a PRIORITY frame is 9 octets.  A PRIORITY frame with any other
+length MUST be treated as a connection error of type HTTP_MALFORMED_PRIORITY.
+
+
+### CANCEL_REQUEST {#frame-cancel-request}
+
+The CANCEL_REQUEST frame (type=0x3) is used to request cancellation of a request
+prior to the response stream being created.  This frame can be used to cancel
+server push requests that are initiated with a PUSH_PROMISE.
+
+The frame identifies a server push request is identified by Push ID (see

There is a typo in here somewhere.. "request _that_ is identified by" perhaps?

-- 
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/701#pullrequestreview-52764642

---

From: Martin Thomson <[email-masked]>
Date: Fri, 28 Jul 2017 01:01:21 +0000 (UTC)

@MikeBishop, I considered that.  Getting the ordering guarantees right here is tricky and I think that it's unfair on clients to say "I'm going to promise you something", but not tell them exactly what.

-- 
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/701#issuecomment-318527491

---

From: Martin Thomson <[email-masked]>
Date: Fri, 28 Jul 2017 01:07:19 +0000 (UTC)

@martinthomson pushed 2 commits.

d569fda  Define a Push ID
bbc331b  Correct grammatical error

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/701/files/649810b442037156d55e624dc7b63df3855e89d3..bbc331b4cbb810575bff32191d331532e84ffc54

---

From: Mike Bishop <[email-masked]>
Date: Fri, 28 Jul 2017 16:49:30 +0000 (UTC)

MikeBishop approved this pull request.

All my feedback is editorial -- the core of the change looks good.

-The server push response is conveyed in the same way as a non-server-push
-response, with response headers and (if present) trailers carried by HEADERS
-frames, and response body (if any) carried by DATA frames.
+After the push stream header, a push contains a response ({{request-response}}),
+with response headers and (if present) trailers carried by HEADERS frames sent
+on the control stream, and response body (if any) carried by DATA frames.

Not on the control stream.

 The flags defined are:
 
+  PUSH_PRIORITIZED (0x04):
+  : Indicates that the Prioritized Stream is a server push rather than a
+    request.  If set, this flag indicates that the Prioritized Stream identifies
+    a response stream and the Promise Index field is present.
+
+  PUSH_DEPENDENT (0x02):
+  : Indicates a dependency on a pushed request.  If set, this flag indicates
+    that the Dependent Stream identifies a response stream and that the

Saying that it "identifies a response stream" in both these places feels a little odd.  If it's the stream ID, then just say it's the stream ID and you don't even need the flags.  If it's the Push ID, use the term here for clarity.

> @@ -430,22 +473,87 @@ The flags defined are:
 
 The PRIORITY frame payload has the following fields:
 
-  Prioritized Stream:
-  : A 32-bit stream identifier for the request stream whose priority is being
-    updated.
+  Prioritized Request:
+  : A 32-bit identifier for a request.  This contains the stream ID of a request
+    stream when the PUSH_PRIORITIZED flag is clear, or a Push ID when the PUSH

PUSH => PUSH_PRIORITIZED?

   Weight:
   : An unsigned 8-bit integer representing a priority weight for the stream (see
     {{!RFC7540}}, Section 5.3). Add one to the value to obtain a weight between
     1 and 256.
 
-A PRIORITY frame MUST have a payload length of nine octets.  A PRIORITY frame
-of any other length MUST be treated as a connection error of type
-HTTP_MALFORMED_PRIORITY.
+A PRIORITY frame identifies a request to priotize, and a request upon which that
+request is dependent.  A request is identified by the stream ID of a request
+when the corresponding PUSH_PRIORITIZED or PUSH_DEPENDENT flag is not set.
+Setting the PUSH or PUSH_DEPENDENT flag causes the frame to identify a
+PUSH_PROMISE using a Push ID (see {{frame-push-promise}} for details).
+
+A PRIORITY frame MAY identify no request by using a stream ID of 0; as in

Since there are two references in a PRIORITY frame, and setting one of them to zero is prohibited by your other pull request, perhaps be more specific than "identify" here.

> +{{!RFC7540}}, this makes the request dependent on the root of the dependency
+tree.
+

Given that there aren't (currently 😉) separate request/response streams, this basically can only be used to cancel a push.  Let's call it CANCEL_PUSH for now and not talk about requests in this section.

+server push requests that are initiated with a PUSH_PROMISE.
+
+The CANCEL_REQUEST frame identifies a server push request by Push ID (see
+{{frame-push-promise}}).
+
+When a server receives this frame, it aborts sending the response for the
+identified server push.  If the server has not yet started to send the server
+push, it can use the receipt of a CANCEL_REQUEST frame to avoid opening a
+stream.  If the push stream has been opened by the server, the server SHOULD
+sent a QUIC RST_STREAM frame on those streams and cease transmission of the
+response.
+
+A server can send this frame to indicate that it won't be sending a response
+prior to creation of a push stream.  Once the push stream has been created, a
+RST_STREAM with a CANCELLED code can be used instead.

"can" feels like it needs an RFC2119 WORD, but I don't know which.

> +The CANCEL_REQUEST frame identifies a server push request by Push ID (see
+{{frame-push-promise}}).
+
+When a server receives this frame, it aborts sending the response for the
+identified server push.  If the server has not yet started to send the server
+push, it can use the receipt of a CANCEL_REQUEST frame to avoid opening a
+stream.  If the push stream has been opened by the server, the server SHOULD
+sent a QUIC RST_STREAM frame on those streams and cease transmission of the
+response.
+
+A server can send this frame to indicate that it won't be sending a response
+prior to creation of a push stream.  Once the push stream has been created, a
+RST_STREAM with a CANCELLED code can be used instead.
+
+A CANCEL_REQUEST frame is sent on the control stream.  Sending a CANCEL_REQUEST
+frame on a stream other than the control stream MUST be treated as a

"stream error of type X" avoids the argument of whether to do a/an before HTTP.

> +RST_STREAM with a CANCELLED code can be used instead.
+
+A CANCEL_REQUEST frame is sent on the control stream.  Sending a CANCEL_REQUEST
+frame on a stream other than the control stream MUST be treated as a
+HTTP_WRONG_STREAM error.
+
+The CANCEL_REQUEST frame has no defined flags.
+
+~~~~~~~~~~  drawing
+    0                   1                   2                   3
+    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+   |                         Push ID (32)                          |
+   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+~~~~~~~~~~
+{: #fig-cancel-request title="CANCEL_REQUEST frame payload"}

I've been trying to avoid drawings of things that have only a single field and call that single thing the payload of the frame.

> @@ -577,12 +686,35 @@ server to client, as in HTTP/2.  It defines no flags.
 
 The payload consists of:
 
-  Promised Stream ID:
-  : A 32-bit Stream ID indicating the QUIC stream on which the response will be
-    sent
-
-  Payload:
-  : HPACK-compressed request headers for the promised response.
+Push ID:
+: A 32-bit identifier for the server push request.  This identifier is used in
+  push stream header ({{server-push}}), CANCEL_REQUEST frames

"in the" or "in ... headers".  "In ... header" sounds odd.

-- 
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/701#pullrequestreview-52978340

---

From: Mike Bishop <[email-masked]>
Date: Fri, 28 Jul 2017 09:51:06 -0700

Oh, and fill in the changelog.

-- 
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/701#issuecomment-318705528

---

From: Martin Thomson <[email-masked]>
Date: Mon, 31 Jul 2017 01:35:05 +0000 (UTC)

martinthomson commented on this pull request.

Heh, you noticed that, but then you didn't notice that the ordering was completely wrong :)

-- 
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/701#discussion_r130260042

---

From: Martin Thomson <[email-masked]>
Date: Sun, 30 Jul 2017 18:37:05 -0700

The flags defined are:

I think that I can remove the second sentence and rely on the description of the fields below.

-- 
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/701#discussion_r130260138

---

From: Martin Thomson <[email-masked]>
Date: Sun, 30 Jul 2017 18:45:16 -0700

I've gone with SHOULD.  There isn't any point in MUST here, because it's not a correctness issue, it's just pointless to use CANCEL_PUSH at this point.

-- 
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/701#discussion_r130260530