Re: [quicwg/base-drafts] Rework flow of Push ID (#3925)

Bence Béky <notifications@github.com> Tue, 21 July 2020 17:38 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 2EAA53A0C85 for <quic-issues@ietfa.amsl.com>; Tue, 21 Jul 2020 10:38:31 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3.101
X-Spam-Level:
X-Spam-Status: No, score=-3.101 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, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, RCVD_IN_MSPIKE_H2=-0.001, 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 pk-05dW_97BU for <quic-issues@ietfa.amsl.com>; Tue, 21 Jul 2020 10:38:29 -0700 (PDT)
Received: from out-27.smtp.github.com (out-27.smtp.github.com [192.30.252.210]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 5E8CB3A0C86 for <quic-issues@ietf.org>; Tue, 21 Jul 2020 10:38:29 -0700 (PDT)
Received: from github-lowworker-0f78100.ash1-iad.github.net (github-lowworker-0f78100.ash1-iad.github.net [10.56.25.48]) by smtp.github.com (Postfix) with ESMTP id 4014BE0654 for <quic-issues@ietf.org>; Tue, 21 Jul 2020 10:38:28 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1595353108; bh=ryvVoT+nX7FJG3Kl3o1AI/th9gw5Yx/Y6xQKJOeLA0I=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=xuaK/m8mf/ArdvHbAahzY84aTNDYv5QZmMkZsQiSsWTkWZbMF5P9JlqNqfTG+HM7L GizCm+lvfoFjrD8gDb2Nj0zBEsWsrh9iF+KpukW1CwlXzvO9XKq7pa283wD7B/BukB aCCKTGlC2R3SYEBqfHVIdKuXr2DWerB1Yzxc+qGY=
Date: Tue, 21 Jul 2020 10:38:28 -0700
From: Bence Béky <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK3HBNIO62ZX77BYOEF5EMERJEVBNHHCPBRDMY@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3925/review/452672403@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3925@github.com>
References: <quicwg/base-drafts/pull/3925@github.com>
Subject: Re: [quicwg/base-drafts] Rework flow of Push ID (#3925)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5f172814313df_148b3fc7a2ccd964166589"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: bencebeky
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/QvLX5BIAZx3Hk6D3UZDICoCGpG4>
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: Tue, 21 Jul 2020 17:38:31 -0000

@bencebeky requested changes on this pull request.

LGTM with minor nits

> -same order, and both the name and the value in each field MUST be exact
-matches.
-
-Server push is only enabled on a connection when a client sends a MAX_PUSH_ID
-frame; see {{frame-max-push-id}}. A server cannot use server push until it
-receives a MAX_PUSH_ID frame. A client sends additional MAX_PUSH_ID frames to
-control the number of pushes that a server can promise. A server SHOULD use Push
-IDs sequentially, starting at 0. A client MUST treat receipt of a push stream
-with a Push ID that is greater than the maximum Push ID as a connection error of
-type H3_ID_ERROR.
+Each server push is identified by a unique Push ID, which is used to refer to
+the push in various contexts throughout the lifetime of the connection.
+
+The Push ID space begins at zero, and ends at a maximum value set by the
+MAX_PUSH_ID frame; see {{frame-max-push-id}}.  Server push is only enabled on a
+connection when a client sends a MAX_PUSH_ID frame.  A client sends additional

"Server push is only enabled on a connection when a client sends a MAX_PUSH_ID frame."  I know this sentence is identical to what it was before, but I have minor issues with it.  First, after "a connection" is introduced, I believe it should be "the client" instead of "a client", because "a connection" only has one client.  Second, "after" would be more appropriate than "when", because server push is enabled not only at the moment of the client sending the frame, but for the rest of the connection.


> -
-Server push is only enabled on a connection when a client sends a MAX_PUSH_ID
-frame; see {{frame-max-push-id}}. A server cannot use server push until it
-receives a MAX_PUSH_ID frame. A client sends additional MAX_PUSH_ID frames to
-control the number of pushes that a server can promise. A server SHOULD use Push
-IDs sequentially, starting at 0. A client MUST treat receipt of a push stream
-with a Push ID that is greater than the maximum Push ID as a connection error of
-type H3_ID_ERROR.
+Each server push is identified by a unique Push ID, which is used to refer to
+the push in various contexts throughout the lifetime of the connection.
+
+The Push ID space begins at zero, and ends at a maximum value set by the
+MAX_PUSH_ID frame; see {{frame-max-push-id}}.  Server push is only enabled on a
+connection when a client sends a MAX_PUSH_ID frame.  A client sends additional
+MAX_PUSH_ID frames to control the number of pushes that a server can promise.  A
+server SHOULD use Push IDs sequentially.  A client MUST treat receipt of a push

It might be beneficial to keep the "starting with 0" phrase at the end of the sentence, because the first sentence of this paragraph does not explicitly prescribe that a server SHOULD start with zero.

> -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
-fields, 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 field sections MUST contain the same fields in the
-same order, and both the name and the value in each field MUST be exact
-matches.
-
-Server push is only enabled on a connection when a client sends a MAX_PUSH_ID
-frame; see {{frame-max-push-id}}. A server cannot use server push until it
-receives a MAX_PUSH_ID frame. A client sends additional MAX_PUSH_ID frames to
-control the number of pushes that a server can promise. A server SHOULD use Push
-IDs sequentially, starting at 0. A client MUST treat receipt of a push stream
-with a Push ID that is greater than the maximum Push ID as a connection error of
-type H3_ID_ERROR.
+Each server push is identified by a unique Push ID, which is used to refer to

It might be beneficial to state that it is the server that assigns Push IDs to pushes (even though it is not only that server that can use Push IDs to refer to pushes).

-- 
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/3925#pullrequestreview-452672403