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

Mike Bishop <notifications@github.com> Fri, 27 December 2019 16:03 UTC

Return-Path: <noreply@github.com>
X-Original-To: quic-issues@ietfa.amsl.com
Delivered-To: quic-issues@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 45438120098 for <quic-issues@ietfa.amsl.com>; Fri, 27 Dec 2019 08:03:55 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -8
X-Spam-Level:
X-Spam-Status: No, score=-8 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, RCVD_IN_DNSWL_HI=-5, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=github.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id A02mer_-3GxD for <quic-issues@ietfa.amsl.com>; Fri, 27 Dec 2019 08:03:52 -0800 (PST)
Received: from out-20.smtp.github.com (out-20.smtp.github.com [192.30.252.203]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 98F7312008F for <quic-issues@ietf.org>; Fri, 27 Dec 2019 08:03:52 -0800 (PST)
Date: Fri, 27 Dec 2019 08:03:51 -0800
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1577462632; bh=K04O1EwXf7u1768ZRBonm7e6tPT/iLaSUY3vD9RV0vw=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=DmqRDiPFmwiE7hCVjEf9zZdEA3gkN3k752bGnA9xccfd+Qi3SNqnXaZ3TlHHsXyXj knVvDYMX4Mw/BCFkIm/gLBn76tASBtuM58iCaQTPi6bYVBD4TYCOpacr3mVREaOL7s cEjE/rgyGMrM48aqTXr1ENR84l0QeDY6kXEfs8/4=
From: Mike Bishop <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK5VWPLUBGGIFJ4AA2N4CNO6PEVBNHHCAQAPGY@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3309/review/336806725@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3309@github.com>
References: <quicwg/base-drafts/pull/3309@github.com>
Subject: Re: [quicwg/base-drafts] Remove DUPLICATE_PUSH and allow duplicate PUSH_PROMISE (#3309)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5e062b67e6f83_47a33f89bf8cd95c8472f9"; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: MikeBishop
X-GitHub-Recipient: quic-issues
X-GitHub-Reason: subscribed
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address: quic-issues@ietf.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/10bgQiRs2PUa4d6aVl90_C9FkMI>
X-BeenThere: quic-issues@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Notification list for GitHub issues related to the QUIC WG <quic-issues.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/quic-issues/>
List-Post: <mailto:quic-issues@ietf.org>
List-Help: <mailto:quic-issues-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 27 Dec 2019 16:03:55 -0000

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