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

Ryan Hamilton <notifications@github.com> Fri, 10 January 2020 23: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 D161412008C for <quic-issues@ietfa.amsl.com>; Fri, 10 Jan 2020 15:03:27 -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 WJeTeEuP9G41 for <quic-issues@ietfa.amsl.com>; Fri, 10 Jan 2020 15:03:24 -0800 (PST)
Received: from out-18.smtp.github.com (out-18.smtp.github.com [192.30.252.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B10E0120125 for <quic-issues@ietf.org>; Fri, 10 Jan 2020 15:03:24 -0800 (PST)
Date: Fri, 10 Jan 2020 15:03:23 -0800
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1578697403; bh=HwukngKG/e8A3toQixiGl7u+lb55Fa2owuutNO5GPCo=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=inagFqzP4rPBwH3lJSf+wtSBX39aHNcPl0ltp3ffrPOOSxeJ7vSWEhJmjPtWNES5c roK9Ur9FrwOWupY5VyYvJdv2xHzpQYRdpE6M8MhXWlwJPA06sSijfkK9+Fmw7JsZqT 40TYOpXS2A3mHvraTbwMZznktIUgn7BSZpGYkxwc=
From: Ryan Hamilton <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJKYYKM5MLCGZGSOWLVV4EY2TXEVBNHHCAQAPGY@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/341467049@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_5e1902bb50d9d_9903fd1d18cd95c115030"; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: RyanAtGoogle
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/iIZ8zo0yv7B_q6-rC5HI4jN2K2U>
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, 10 Jan 2020 23:03:28 -0000

RyanAtGoogle commented on this pull request.



> @@ -1238,9 +1232,20 @@ 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 header sets MUST contain the same fields in the same
+order, and both the name and and value in each field MUST be exact
+matches. If a client receives a Push ID that has already been promised
+and detects a mismatch, it MUST respond with a connection error of type
+H3_GENERAL_PROTOCOL_ERROR. If the decompressed header sets match exactly, the
+client MUST ignore the duplicate PUSH_PROMISE frame.

> Yes, I think we've found a conceptual difference. You're looking at the promise purely as a way to forestall requests for that resource, and the fact that it happens to be on a given stream is only because the server thinks there might be something on that stream where you need to know about the push first. But handling of push in Chrome is global -- if you request A and B, I promise X on A, and B references X, you'll use the pushed object if you know about it. I agree, in this implementation style, the first PUSH_PROMISE frame to arrive carries all the information you need, and any subsequent ones are basically irrelevant.
> 
> But RFC7540 says that "Pushed responses are always associated with an explicit request from the client." How (or whether, obviously!) that association is used is implementation-dependent, but I think for consistency's sake we need to provide that association in the document. (As far as I know, the only thing H2 explicitly used this association for internally was for prioritization -- pushed streams depend on the associated stream.)

Interesting! I read the language in RFC7540 differently, I think. It says "Pushed responses are always associated with an explicit request from the client.  The PUSH_PROMISE frames sent by the server are sent on that explicit request's stream. " To me, this language exists to prevent unilateral pushes by the server. In other words, if there are no client-initiated requests outstanding, the server may not push any resources. 

But if I understand you correctly, you read it as enabling implementations to make use of this association for more than simply preventing unilateral pushes. (Presumably this association could be used by an implementation to "hide" the pushed resource from a subsequent request not associated with the original request, but to "show" it to a subsequent request that *was* associated with the original request? I would not have thought this was "a thing".) Fascinating.

> Do we need a separate editorial issue to expand on this a bit in the document, or is it there and we're just reading through different filters?

This is a good question :) I think some editorial exposition on this front would be welcome! I would be very curious to hear what other folks think of this issue.

In any case, backing up to the change you proposed:

> client SHOULD associate the pushed content with each stream on which a PUSH_PROMISE was received.

I think I'm comfortable with this text even though I'm slightly uneasy about what "associate" means here. But that's pretty orthogonal to this PR since it existed in the DUPLICATE_PUSH language as well, so I'll go ahead and apply your suggestion.

Thanks for the great discussion!

-- 
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#discussion_r365464586