Subject: [quicwg/base-drafts] QPACK [editorial] Misc minor editorial changes. (#2942)

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 09:54:33 -0700

You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/2942

-- Commit Summary --

  * non-controversial one-offs

-- File Changes --

    M draft-ietf-quic-qpack.md (200)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2942.patch
https://github.com/quicwg/base-drafts/pull/2942.diff

-- 
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/2942

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 09:55:02 -0700

bencebeky commented on this pull request.

> @@ -84,7 +84,7 @@ code and issues list for this draft can be found at
 
 The QUIC transport protocol {{QUIC-TRANSPORT}} is designed to support HTTP
 semantics, and its design subsumes many of the features of HTTP/2 {{?RFC7540}}.
-HTTP/2 uses HPACK ({{!RFC7541}}) for header compression.  If HPACK were used for

No other reference in this paragraph is enclosed in parentheses.

-- 
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/2942#pullrequestreview-269149538

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 09:55:41 -0700

> @@ -192,7 +192,7 @@ while the decoder is relatively simple.
 
 An encoder MUST ensure that a header block which references a dynamic table
 entry is not processed by the decoder after the referenced entry has been
-evicted.  Hence the encoder needs to track information about each compressed

Once a header block is sent, this collection of information does not change.  This makes me believe "retain" is a more fitting word than "track".

-- 
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/2942#pullrequestreview-269149865

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 09:56:30 -0700

> @@ -272,25 +270,26 @@ An encoder can decide whether to risk having a stream become blocked. If
 permitted by the value of SETTINGS_QPACK_BLOCKED_STREAMS, compression efficiency
 can often be improved by referencing dynamic table entries that are still in
 transit, but if there is loss or reordering the stream can become blocked at the
-decoder.  An encoder avoids the risk of blocking by only referencing dynamic

"Can" better expresses that the encoder has a choice.

-- 
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/2942#pullrequestreview-269150286

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 09:57:34 -0700

>  table entries which have been acknowledged, but this could mean using
 literals. Since literals make the header block larger, this can result in the
 encoder becoming blocked on congestion or flow control limits.
 
 ### Known Received Count
 
-In order to identify which dynamic table entries can be safely used without a
-stream becoming blocked, the encoder tracks the number of entries received by

Reworded to include that the decoder also needs to track the Known Received Count.

-- 
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/2942#pullrequestreview-269150945

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 09:59:36 -0700

-When blocking references are permitted, the encoder uses Header Acknowledgement
-instructions ({{header-acknowledgement}}) to maintain the Known Received
-Count.  If a header block was potentially blocking, the acknowledgement implies
-that the decoder has received all dynamic table state necessary to process the
-header block.  If the Required Insert Count of an acknowledged header block was
-greater than the encoder's current Known Received Count, the block's Required
-Insert Count becomes the new Known Received Count.

"the block's Required Insert Count becomes the new Known Received Count" always trips me: does the value of the Required Insert Count change to the value of the Known Received Count, or does the value of the Required Insert Count became to be known as the new value of the Known Received Count?

-- 
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/2942#pullrequestreview-269152016

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:00:34 -0700

> @@ -430,12 +431,9 @@ Dynamic table entries can have empty values.
 
 The size of the dynamic table is the sum of the size of its entries.
 
-The size of an entry is the sum of its name's length in bytes (as defined in
-{{string-literals}}), its value's length in bytes, and 32.

The reference to {{string-literals}} here is misleading, because length in that section can mean length after Huffman encoding.

-- 
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/2942#pullrequestreview-269152510

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:01:46 -0700

> @@ -445,19 +443,23 @@ encoder sends a Set Dynamic Table Capacity instruction
 ({{set-dynamic-capacity}}) with a non-zero capacity to begin using the dynamic
 table.
 
-Before a new entry is added to the dynamic table, entries are evicted from the

I am not aware of any reason the specs need to require an implementation to evict first, then add.  Spelling out a choice also allows the caution on use-after-free to be restricted to one of the two methods.

-- 
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/2942#pullrequestreview-269153148

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:02:50 -0700

> @@ -477,28 +479,27 @@ it can choose to use a lower dynamic table capacity (see
 
 For clients using 0-RTT data in HTTP/3, the server's maximum table capacity is
 the remembered value of the setting, or zero if the value was not previously
-sent.  When the client's 0-RTT value of the SETTING is 0, the server MAY set it

To me, zero looks better written in numbers, especially with that many occurrences of "non-zero" in the same section.

-- 
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/2942#pullrequestreview-269153684

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:04:58 -0700

> @@ -586,8 +588,10 @@ d = count of entries dropped
 ### Prefixed Integers
 
 The prefixed integer from Section 5.1 of [RFC7541] is used heavily throughout
-this document.  The format from [RFC7541] is used unmodified.  QPACK
-implementations MUST be able to decode integers up to 62 bits long.
+this document.  The format from [RFC7541] is used unmodified.  Note, however,
+that QPACK uses some prefix sizes not actually used in HPACK.

I was actually tripped by this when re-using the HPACK implementation, because that had DCHECKs on prefix length that restricted it to values that occur in HPACK.  And of course there was no test coverage for other prefixes.  In particular, (3+) and (8+) occur in the current QPACK draft but not in RFC7541.

-- 
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/2942#pullrequestreview-269154750

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:07:13 -0700

@bencebeky pushed 1 commit.

c049ed1fbfedda303ef6ca024982f503bdbc3b08  Do not unnecessarily change "avoid" to "choose not".

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942/files/7155dcc0b47f9a1ca99e59e45777f4f8f7baeb98..c049ed1fbfedda303ef6ca024982f503bdbc3b08

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:09:16 -0700

> @@ -627,14 +632,14 @@ type HTTP_WRONG_STREAM_COUNT. These streams MUST NOT be closed. Closure of
 either unidirectional stream type MUST be treated as a connection error of type
 HTTP_CLOSED_CRITICAL_STREAM.
 
-An endpoint MAY avoid creating its own encoder stream if it's not going to be

I do not have the impression that HTTP/3 defines ownership of unidirectional streams, but "create" is unambiguous anyway.

-- 
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/2942#pullrequestreview-269157005

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:09:48 -0700

-An endpoint MAY avoid creating its own encoder stream if it's not going to be
-used (for example if the endpoint doesn't wish to use the dynamic table, or if

Each endpoint has two contexts and each context has a dynamic table, so it's better to clarify which one.

-- 
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/2942#pullrequestreview-269157306

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:10:54 -0700

-An endpoint MAY avoid creating its own decoder stream if the maximum size of
-its own dynamic table is zero.

I find that ownership of a dynamic table, while not necessarily ambiguous, is at least undefined.

-- 
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/2942#pullrequestreview-269157903

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:11:16 -0700

-An endpoint MUST allow its peer to create both encoder and decoder streams

"both encoder and decoder streams" might give the reader the impression that there could be multiple of each.

-- 
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/2942#pullrequestreview-269158090

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:12:15 -0700

> @@ -741,20 +746,16 @@ the existing entry is represented as an integer with a 5-bit prefix.
 ~~~~~~~~~~
 {:#fig-index-with-duplication title="Duplicate"}
 
-The existing entry is re-inserted into the dynamic table without resending
-either the name or the value. This is useful to mitigate the eviction of older

I find "mitigate the eviction" confusing.

-- 
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/2942#pullrequestreview-269158666

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:14:54 -0700

7bc269a3b73015041ad2e8ea95164aff14d64f49  Do not unnecessarily remove "without resending..." phrase.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2942/files/c049ed1fbfedda303ef6ca024982f503bdbc3b08..7bc269a3b73015041ad2e8ea95164aff14d64f49

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:15:57 -0700

## Decoder Instructions {#decoder-instructions}
 
-Decoder instructions provide information used to ensure consistency of the

I propose to change this paragraph to match the first paragraph of Encoder Instructions section.

-- 
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/2942#pullrequestreview-269160718

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:17:06 -0700

-Decoder instructions provide information used to ensure consistency of the
-dynamic table. They are sent from the decoder to the encoder on a decoder
-stream; that is, the server informs the client about the processing of the

This discussion of client and server seems out of place.  If it is really necessary, Section 2 might be a better place to talk about the two contexts and how both endpoint needs to have an encoder and a decoder.

-- 
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/2942#pullrequestreview-269161294

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:17:44 -0700

> @@ -814,7 +815,7 @@ cause a stream to become blocked, as described in {{state-synchronization}}.
 ~~~~~~~~~~
 {:#fig-size-sync title="Insert Count Increment"}
 
-An encoder that receives an Increment field equal to zero or one that increases

"zero or one" gets the reader's brain in the wrong groove.

-- 
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/2942#pullrequestreview-269161620

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:19:59 -0700

> @@ -830,10 +831,8 @@ modifying it.
 
 Each header block is prefixed with two integers.  The Required Insert Count is
 encoded as an integer with an 8-bit prefix after the encoding described in
-{{ric}}).  The Base is encoded as sign-and-modulus integer, using a single sign

A cursory web search shows that "sign-and-magnitude" is used a lot more often than "sign-and-modulus", which might conjure images of modulo arithmetic.  I propose not to put a name on this encoding, since there's an off-by-one that's not common for sign-and-modulus representations anyway.

-- 
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/2942#pullrequestreview-269162743

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:20:45 -0700

> @@ -937,11 +939,11 @@ entries, the Base will be greater than the Required Insert Count, so the delta
 will be positive and the sign bit is set to 0.
 
 An encoder that produces table updates before encoding a header block might set
-Required Insert Count and the Base to the same value.  In such case, both the

Rewording to emphasize that the encoder has very limited freedom in what to set Required Insert Count to.

-- 
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/2942#pullrequestreview-269163135

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:21:09 -0700

> @@ -993,7 +993,7 @@ field, represented as an integer with a 4-bit prefix (see Section 5.1 of
 
 ### Literal Header Field With Name Reference {#literal-name-reference}
 
-A literal header field with a name reference represents a header where the

Removing "a" to be consistent with subsection title.

-- 
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/2942#pullrequestreview-269163357

---

From: Bence Béky <[email-masked]>
Date: Wed, 31 Jul 2019 10:21:59 -0700

> @@ -1119,7 +1119,7 @@ HTTP_QPACK_DECODER_STREAM_ERROR (0x202):
 
 # Security Considerations
 
-TBD.
+TBD.  Also see Section 7.1 of [RFC7541].

This allows the reference to Section 7.1 in Literal Header Field With Name Reference section be replaced with an internal reference to this section.

-- 
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/2942#pullrequestreview-269163805