Subject: [quicwg/base-drafts] Robert Wilton's QPACK Comment 3 (#4802)

From: Lucas Pardue <[email-masked]>
Date: Thu, 21 Jan 2021 06:29:55 -0800

@rgwilton said

> Section 7.4 talks about implementation limits, but it wasn't obvious to me how
> a receiver is expected to behave if one of those limits is exceeded.  Further
> in the case of strings, there seems to be a simple upper bound on the maximum
> size of the string of the table size.

-- 
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/issues/4802

---

From: afrind <[email-masked]>
Date: Mon, 25 Jan 2021 16:44:22 -0800

Section 3.2.2 mentions that it is an error to attempt to insert an entry larger than the table size: 

> It is an error if the encoder attempts to add an entry that is larger than the dynamic table capacity; the decoder MUST treat this as a connection error of type QPACK_ENCODER_STREAM_ERROR.

I can't actually find a place where the document says that if you fail to decode an instruction or reference (for any reason, which would include exceeding implementation limits) that it's an error.  I think we might need 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/issues/4802#issuecomment-767205456

---

From: afrind <[email-masked]>
Date: Tue, 26 Jan 2021 09:40:45 -0800

Actually, section 6 says:

> QPACK_DECOMPRESSION_FAILED (0x200):
    The decoder failed to interpret an encoded field section and is not able to continue decoding that field section. 
QPACK_ENCODER_STREAM_ERROR (0x201):
    The decoder failed to interpret an encoder instruction received on the encoder stream. 
QPACK_DECODER_STREAM_ERROR (0x202):
    The encoder failed to interpret a decoder instruction received on the decoder stream.
    
So "failing to interpret" anything results in an error.  I think this includes being unable to decode an integer or string because of implementation limits.

Is this sufficient or should we add something a bit more explicit?

-- 
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/issues/4802#issuecomment-767709156

---

From: Robert Wilton <[email-masked]>
Date: Tue, 26 Jan 2021 10:37:07 -0800

If section 3.2.2 already imposes a limit on the length of an entry, then it would be good to referenced that text from section 7.4.

E.g. section 7.4 states:  `In
   the same way, it has to set a limit to the length it accepts for
   string literals; see Section 4.1.2.`

But I suspect that really the only limit that is needed here is the one in 3.2.2?  I.e. it seems reasonable that a receiver should accept any string up to the maximum table size?

But my other point is that it suggests that a receiver can arbitrarily choose to set a limit on the size of integers that it receives, but section 4.1.1 states: `   QPACK implementations MUST be able to decode integers up to and
   including 62 bits long.`

So, I guess that I'm really suggesting that this paragraph (in section 7.4):

`   An implementation has to set a limit for the values it accepts for
   integers, as well as for the encoded length; see Section 4.1.1.  In
   the same way, it has to set a limit to the length it accepts for
   string literals; see Section 4.1.2.`

is reworded to say something along the lines of:
- Receivers MUST accept integers up to 62 bits long section 4.1.1, but MAY choose to not handle larger integers, in which case they return QPACK_DECOMPRESSION_FAILED.
- Receivers MUST accept any strings up to maximum table length -32, or return QPACK_DECOMPRESSION_FAILED.

It wasn't clear to me whether integers bigger than 62 bits are ever likely in practice, I presume not ...

-- 
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/issues/4802#issuecomment-767744393

---

From: Dmitri Tikhonov <[email-masked]>
Date: Tue, 26 Jan 2021 13:10:25 -0800

A decoder that actually counts insertions (instead of using modulo arithmetic) will likely fail when the integer value it uses overflows.

Of course, one does not have to use `uint32_t` or `uint64_t` for the count.  To avoid an overflow, there are math libraries that support arbitrarily large numbers.

-- 
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/issues/4802#issuecomment-767829630

---

From: Mike Bishop <[email-masked]>
Date: Tue, 26 Jan 2021 15:30:15 -0800

The text is really attempting to say that an implementation MUST NOT continue to process to infinity; bounds must exist for basic sanity.  What those bounds will be is implementation-dependent.  Saying that they're limited to the table size is only true on the encoder stream; request streams can have string literals greater than the table size (consider the degenerate case of zero).

HTTP limits could be pushed down to QPACK such that the library will refuse to decode a larger string than the implementation would accept; I don't think that's necessarily required though.

-- 
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/issues/4802#issuecomment-767898012

---

From: Robert Wilton <[email-masked]>
Date: Wed, 27 Jan 2021 02:04:51 -0800

I don't think that using arbitrarily sized integer libraries is the answer here due to the performance overheads of such libraries.

My concerns are two fold:

(1) It is unclear what arbitrary limit receivers should apply.  E.g., is 255 bytes enough for a string?  Is a 64 bit signed integer enough for an integer?

(2) Senders probably won't really know that the reason the stream has failed is because they are hitting some arbitrary limit in the receiver, and even if they did then I'm not sure there is anything that they can do about it.

Hence, another suggestion is to specifying minimum expected bounds for these values?  Or if these bounds are effectively provided by the HTTP spec, perhaps the appropriate section in that spec. could be referenced.

-- 
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/issues/4802#issuecomment-768175712

---

From: Lars Eggert <[email-masked]>
Date: Thu, 28 Jan 2021 04:09:15 -0800

Editors, could we come to a conclusion here?

-- 
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/issues/4802#issuecomment-769010731

---

From: afrind <[email-masked]>
Date: Thu, 28 Jan 2021 07:29:59 -0800

There is already a requirement that implementations be able to decode 62 bit integers.  Anything larger is really necessary -- we intentionally encode the "Required Insert Count" on the wire in a way that bounds its size to be much smaller practically.  The requirement stems from the need to decode QUIC stream IDs.  I think @dtikhonov's comment about bignums is about what an implementation might do if it overflowed its internal representation decoding the Required Insert Count.  This is not a practical concern, and can be completely worked around with a "clever" implementation that is worried about more than 2^64 table insertions.

HTTP also provides a setting to inform the peer of a maximum size of a field section (MAX_FIELD_SECTION_SIZE -- see HTTP/3 section 4.1.1.3), which is also a practical upper bound for any single QPACK string for HTTP/3.  Perhaps the QPACK doc could provide this hint?  I'll draft a PR to this effect.

-- 
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/issues/4802#issuecomment-769164722

---

From: Martin Thomson <[email-masked]>
Date: Thu, 28 Jan 2021 16:39:16 -0800

FWIW, exceeding 2^60 insertions would be a tall order given data limits in QUIC; a 64-bit integer is definitely enough space for any size or length field.  Implementations can use smaller values, probably reliably.  There is some need for care in those cases, but I don't see the spec as bearing any responsibility for that.

@afrind, remember that MAX_FIELD_SECTION_SIZE is advisory, so I wouldn't go so far as to say "practical".

-- 
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/issues/4802#issuecomment-769492369