Re: [quicwg/base-drafts] QPACK [editorial] Update text on instructions and representations. (#2941)

afrind <notifications@github.com> Tue, 13 August 2019 21:58 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 5AF361208FE for <quic-issues@ietfa.amsl.com>; Tue, 13 Aug 2019 14:58:50 -0700 (PDT)
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 MHKMd-pSwRJb for <quic-issues@ietfa.amsl.com>; Tue, 13 Aug 2019 14:58:45 -0700 (PDT)
Received: from out-24.smtp.github.com (out-24.smtp.github.com [192.30.252.207]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D5674120926 for <quic-issues@ietf.org>; Tue, 13 Aug 2019 14:58:44 -0700 (PDT)
Date: Tue, 13 Aug 2019 14:58:43 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1565733524; bh=uxaNjgUcfDK79xgBvJ+IgAZ4aJpRcyg6++tzVQH4bBY=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=YploE7c6jVHN/9QdwemnG5Gn64ejdMqLbe2655oDgBXxuz3SPSskZ2a4BwgA+oAnQ M+vlTwqFP/TcNwBL4EY40I4MYC5zjqz4LY0oaoIE8K3fIv4Kw7+eZVDim0dKFK1h8c t5JugJLRM9rSjqCJuQdpSxzWilACs+47uw1WQZuQ=
From: afrind <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK75ZB57PH7ZSLJAK2V3MBSRHEVBNHHBYU5PJU@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/2941/review/274577499@github.com>
In-Reply-To: <quicwg/base-drafts/pull/2941@github.com>
References: <quicwg/base-drafts/pull/2941@github.com>
Subject: Re: [quicwg/base-drafts] QPACK [editorial] Update text on instructions and representations. (#2941)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5d533293f02e3_6d183fb1c38cd96c221221"; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: afrind
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/YFRlfZ7okbCzmKi0npt7yAvxXRc>
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, 13 Aug 2019 21:59:02 -0000

afrind commented on this pull request.



> @@ -821,10 +828,13 @@ connection error of type `HTTP_QPACK_DECODER_STREAM_ERROR`.
 
 ## Header Block Representations
 
-Header blocks contain compressed representations of header lists and are carried
-in frames on streams defined by the enclosing protocol.  These representations
-reference the static table, or dynamic table in a particular state without
-modifying it.
+A header block consists of a prefix and a possibly empty sequence of
+representations defined in this section.  Each representation corresponds to a
+single header field.  These representations can reference the static table, or
+the dynamic table in a particular state without modifying it.

Wondering if we still need 'without modifying it'.  True HPACK instructions both emitted header fields and modified the state, but it might be redundant in the context of this document, and it makes it read awkwardly.

> @@ -968,8 +978,8 @@ decoded header list, as described in Section 3.2 of [RFC7541].
 If the entry is in the static table, or in the dynamic table with an absolute
 index less than the Base, this representation starts with the '1' 1-bit pattern,
 followed by the `S` bit indicating whether the reference is into the static or
-dynamic table.  The 6-bit prefix integer (see Section 5.1 of [RFC7541]) that
-follows is used to locate the table entry for the header name.  When S=1, the
+dynamic table.  The 6-bit prefix integer (see {{prefixed-integers}}) that
+follows is used to locate the table entry for the header field.  When S=1, the

nice catch!

>  The following bit, 'N', indicates whether an intermediary is permitted to add
 this header to the dynamic header table on subsequent hops. When the 'N' bit is
-set, the encoded header MUST always be encoded with a literal representation. In
-particular, when a peer sends a header field that it received represented as a

It was copied verbatim from HPACK.  Maybe we should leave it in.

>  used to locate the table entry for the header name.  When S=1, the number
 represents the static table index; when S=0, the number is the relative index of
-the entry in the dynamic table.
+the entry in the dynamic table.  Only the header field name stored in the static
+or dynamic table is used. Any header field value MUST be ignored.

I feel like 'Any header field value MUST be ignored' might be confusing since it doesn't say the header field value from the static or dynamic table -- this representation also contains a value.

-- 
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/2941#pullrequestreview-274577499