Re: [quicwg/base-drafts] Remove DUPLICATE_PUSH and allow duplicate PUSH_PROMISE (#3309)

[Mike Bishop <[e-mail]>]

MikeBishop requested changes on this pull request.

This basically puts it back to the way it was before, and thereby reintroduces the problems we were trying to get away from then.  Those problems aren't insurmountable, so let's fix them in this PR.

> @@ -379,10 +379,9 @@ An HTTP message (request or response) consists of:
 3. optionally, trailing headers, if present (see {{!RFC7230}}, Section 4.1.2),
    sent as a single HEADERS frame.
 
-A server MAY send one or more PUSH_PROMISE (see {{frame-push-promise}}) or
-DUPLICATE_PUSH (see {{frame-duplicate-push}}) frames before, after, or
-interleaved with the frames of a response message. These PUSH_PROMISE and
-DUPLICATE_PUSH frames are not part of the response; see {{server-push}} for
+A server MAY send one or more PUSH_PROMISE (see {{frame-push-promise}}) frames
+frames before, after, or interleaved with the frames of a response message.

```suggestion
before, after, or interleaved with the frames of a response message.
```

> @@ -603,11 +602,11 @@ client making the indicated request.  This trades off network usage against a
 potential latency gain.  HTTP/3 server push is similar to what is described in
 HTTP/2 {{!HTTP2}}, but uses different mechanisms.
 
-Each server push is identified by a unique Push ID. This Push ID is used in a
-single PUSH_PROMISE frame (see {{frame-push-promise}}) which carries the request
-headers, possibly included in one or more DUPLICATE_PUSH frames (see
-{{frame-duplicate-push}}), then included with the push stream which ultimately
-fulfills those promises.
+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
+headers, 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 header fields MUST be byte-for-byte identical.

Technically, this is probably true, because odds are good the data needs to match regardless of whether you're serializing to text or to an internal object.  But without knowing details of internal objects, I don't know for sure what it means to be byte-for-byte identical.  We might want to frame this in terms of structure and contents instead.

I think what it means is that the header set contains the same set of fields in the same order, and that in each field both the name and and value are byte-for-byte matches.

> @@ -623,31 +622,28 @@ allows the server push to be associated with a client request.  Promised
 requests MUST conform to the requirements in Section 8.2 of {{!HTTP2}}.
 
 The same server push can be associated with additional client requests using a
-DUPLICATE_PUSH frame (see {{frame-duplicate-push}}).
+PUSH_PROMISE frame with the same push id on multiple request stream.

```suggestion
PUSH_PROMISE frame with the same Push ID on multiple request stream.
```

> @@ -1238,9 +1233,10 @@ MAX_PUSH_ID frame ({{frame-max-push-id}}). A client MUST treat receipt of a
 PUSH_PROMISE frame that contains a larger Push ID than the client has advertised
 as a connection error of H3_ID_ERROR.
 
-A server MUST NOT use the same Push ID in multiple PUSH_PROMISE frames. A client
-MUST treat receipt of a Push ID which has already been promised as a connection
-error of type H3_ID_ERROR.
+A server MAY use the same Push ID in multiple PUSH_PROMISE frames. If so, the
+decompressed request headers MUST be idential.  A client MUST treat receipt of a
+Push ID which has already been promised with different headers than the previous
+promise as a connection error of type H3_ID_ERROR.

MUST-error requires that the client keep the request headers for the lifetime of the connection in case it ever sees that Push ID again.  Instead, I think we want to say that it MUST error if it detects a mismatch, and MUST ignore the new PUSH_PROMISE if it has not retained the previous request header set.

-- 
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/3309#pullrequestreview-336806725