Re: [quicwg/base-drafts] QPACK editorial revision (#2759)

[Lucas Pardue <[e-mail]>]

LPardue approved this pull request.

These changes all look helpful. I left some minor comments, that may be more about the spec than the changes (sorry).

A more general subjective opinion. In the HTTP/3 spec, each frame description starts as "The XYZ frame (type=0xABC) is used to do <stuff>". I find the style of describing instructions, "To do a thing, an instruction starting with the pattern ABC is used" a little jarring.

>  
 ## Encoder
 
-An encoder compresses a header list by emitting either an indexed or a literal
-representation for each header field in the list.  References to the static
-table and literal representations do not require any dynamic state and never
-risk head-of-line blocking.  References to the dynamic table risk head-of-line
+An encoder compresses a header list into a header block by emitting either an
+indexed or a literal representation for each header field in the list (see

Does it always compress? I.e. could a dumb encoder make bad choices?

> @@ -327,37 +312,86 @@ instructions received on the encoder stream.
 The decoder MUST emit header fields in the order their representations appear in
 the input header block.
 
+### Blocked Decoding
+
+Upon receipt of a header block, the decoder examines the Required Insert Count.
+When the Required Insert Count is less than or equal to the decoder's Insert
+Count, the header block can be processed immediately.  Otherwise, the stream is

noob clarification question: I think this is the stream on which the header block was delivered, maybe it would help to be explicit?

> @@ -327,37 +312,86 @@ instructions received on the encoder stream.
 The decoder MUST emit header fields in the order their representations appear in
 the input header block.
 
+### Blocked Decoding
+
+Upon receipt of a header block, the decoder examines the Required Insert Count.
+When the Required Insert Count is less than or equal to the decoder's Insert
+Count, the header block can be processed immediately.  Otherwise, the stream is
+blocked.
+
+While blocked, header block data SHOULD remain in the blocked stream's flow
+control window.  A stream becomes unblocked when the Insert Count becomes
+greater than or equal to the Required Insert Count for all header blocks the
+decoder has started reading from the stream.
+<!-- doesn't the stream become unblocked when the encoder receives the acks? -->
+
+If the decoder encounters a header block with a Required Insert Count value
+larger than defined in {{blocked-streams}}, it MAY treat this as a connection

I'm not sure what this value is when I look back. Is the error when Required Insert Count is greater than "one larger than the largest absolute index"?

> +When the Required Insert Count is less than or equal to the decoder's Insert
+Count, the header block can be processed immediately.  Otherwise, the stream is
+blocked.
+
+While blocked, header block data SHOULD remain in the blocked stream's flow
+control window.  A stream becomes unblocked when the Insert Count becomes
+greater than or equal to the Required Insert Count for all header blocks the
+decoder has started reading from the stream.
+<!-- doesn't the stream become unblocked when the encoder receives the acks? -->
+
+If the decoder encounters a header block with a Required Insert Count value
+larger than defined in {{blocked-streams}}, it MAY treat this as a connection
+error of type HTTP_QPACK_DECOMPRESSION_FAILED.  If the decoder encounters a
+header block with a Required Insert Count value smaller than defined in
+{{blocked-streams}}, it MUST treat this as a connection error of type
+HTTP_QPACK_DECOMPRESSION_FAILED as prescribed in {{invalid-references}}.

Would it be clearer to phrase the paragraph something like: "when processing header blocks the decoder expects the Required Insert Count value to exactly match the value defined in {{blocked streams}. If it encounters a smaller value, it MUST treat this as a connection error of type HTTP_QPACK_DECOMPRESSION_FAILED. If it encounters a larger value, it MAY treat this as a connection error of type HTTP_QPACK_DECOMPRESSION_FAILED."

>  
 ### State Synchronization
 
-The decoder instructions ({{decoder-instructions}}) signal key events at the
-decoder that permit the encoder to track the decoder's state.  These events are:
+The decoder signals the following key events by emitting decoder instructions
+({{decoder-instructions}}) on the decoder stream.
+
+#### Completed Processing of a Header Block
+
+When the decoder finishes decoding a header block containing dynamic table
+references, it emits a Header Acknowledgement instruction
+({{header-acknowledgement}}).  The same Stream ID can be identified by multiple
+Header Acknowledgement instructions, as multiple header blocks can be sent on a
+single stream in the case of intermediate responses, trailers, and pushed
+requests.  Since frames carrying header blocks on each stream are received and
+processed in order, this gives the encoder precise feedback on which header
+blocks within a stream have been fully processed.

If I understand this correctly, I offer the follow as a suggested change:

"When the decoder finishes decoding a header block containing dynamic table
references, it emits a Header Acknowledgement instruction
({{header-acknowledgement}}).  A stream may carry multiple header blocks
in the case of intermediate responses, trailers, and pushed
requests. Emitting multiple Header Acknowledgements for the same stream, 
when each frame is completely processed in order, gives the encoder precise
feedback.

> +
+When the decoder finishes decoding a header block containing dynamic table
+references, it emits a Header Acknowledgement instruction
+({{header-acknowledgement}}).  The same Stream ID can be identified by multiple
+Header Acknowledgement instructions, as multiple header blocks can be sent on a
+single stream in the case of intermediate responses, trailers, and pushed
+requests.  Since frames carrying header blocks on each stream are received and
+processed in order, this gives the encoder precise feedback on which header
+blocks within a stream have been fully processed.
+
+#### Abandonment of a Stream
+
+When an endpoint receives a stream reset before the end of a stream or before
+all header blocks are processed on that stream, or when it abandons reading of a
+stream, it generates a Stream Cancellation instruction (see
+{{stream-cancellation}}).  This signals to the encoder that all references to

is this all unacknowledged references?

> +
+When the decoder finishes decoding a header block containing dynamic table
+references, it emits a Header Acknowledgement instruction
+({{header-acknowledgement}}).  The same Stream ID can be identified by multiple
+Header Acknowledgement instructions, as multiple header blocks can be sent on a
+single stream in the case of intermediate responses, trailers, and pushed
+requests.  Since frames carrying header blocks on each stream are received and
+processed in order, this gives the encoder precise feedback on which header
+blocks within a stream have been fully processed.
+
+#### Abandonment of a Stream
+
+When an endpoint receives a stream reset before the end of a stream or before
+all header blocks are processed on that stream, or when it abandons reading of a
+stream, it generates a Stream Cancellation instruction (see
+{{stream-cancellation}}).  This signals to the encoder that all references to

Oh is that what the following paragraph is trying to articulate?

>  
-The decoder chooses when to emit Insert Count Increment instructions (see
+After receiving new table entries on the encoder stream, the decoder chooses
+when to emit Insert Count Increment instructions (see
 {{insert-count-increment}}). Emitting an instruction after adding each new

should it be "this instruction" or could it be *any* instruction type.

>  ### Insert With Name Reference
 
-An addition to the header table where the header field name matches the header
-field name of an entry stored in the static table or the dynamic table starts
-with the '1' one-bit pattern.  The `S` bit indicates whether the reference is to
-the static (S=1) or dynamic (S=0) 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 number represents the static table index; when S=0, the
-number is the relative index of the entry in the dynamic table.
+An encoder adds an entry to the dynamic table where the header field name
+matches the header field name of an entry stored in the static table or the

```suggestion
matches the header field name of an entry stored in the static or the
```

you omit this later 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/2759#pullrequestreview-252255145