Open mtrudel opened 2 weeks ago
@mtrudel thanks for the heads up!
I'm most of the way through this work and I've come to the realization that I think we need something like a protocol_max_table_size
field on table. The need comes from a strict read of https://www.rfc-editor.org/rfc/rfc7541#section-6.3, specifically:
The new maximum size [of a dynamic table size update] MUST be lower than or equal to the limit determined by the protocol using HPACK. A value that exceeds this limit MUST be treated as a decoding error. In HTTP/2, this limit is the last value of the SETTINGS_HEADER_TABLE_SIZE parameter
HPAX currently checks the size of such updates against max_table_size
, which isn't correct. The current implementation prevents dynamic table size update (DTSU) messages from increasing the max table size at all, when the protocol is clear that increases up to the protocol negotiated max value are supported (indeed, this is how DTSUs can be used to empty out the decoder's table per https://www.rfc-editor.org/rfc/rfc7541#section-4.2).
So what we actually need in Table is:
size
to reflect the size of the entries currently in the table (per https://www.rfc-editor.org/rfc/rfc7541#section-4.1)max_table_size
to reflect the maximum allowable size of the table before we end up truncatingprotocol_max_table_size
to reflect an upper bound on max table size changes that will be accepted in DTSU messagesHPAX.resize/2
calls should be setting protocol_max_table_size
(as well as decreasing max_table_size
and truncating entries if necessary). Support for DTSU decoding should compare the given size against protocol_max_table_size
(instead of max_table_size
) when validating the value.
As identified at https://github.com/mtrudel/bandit/issues/392, calling
HPAX.resize/2
does not correctly perform the steps needed to handle HTTP/2 settings updates that change theSETTINGS_MAX_HEADER_LIST_SIZE
value.What the RFCS say
RFC9113§4.3.1 describes the process[^1] of how changes to
SETTINGS_MAX_HEADER_LIST_SIZE
are undertaken in terms of the decoder and encoder ends of an HPACK context:How HPAX should be used (and what it should be doing)
On the encoder, application-level calls to
HPAX.resize/2
should be made when the encoder receives a settings frame from the decoder, with the application passing a value no larger than the value specified in the settings frame.On the decoder, application-level calls to
HPAX.resize/2
should be made when the decoder receives a settings ACK (NOT when the settings frame is sent), with the application passing in the exact value from the settings frame that was acknowledged, possibly evicting records as needed.The decoder should process dynamic table size update messages by first ensuring that the message is not increasing the maximum size of the table. It should then be setting the table's max size to the value indicated, possibly evicting records as needed.
Where HPAX currently falls short
HPAX's current implementation is insufficient in the following two ways:
HPAX.Table.resize/2
does not update the max table size, it only evicts entries to decrease the size of the contents of the table. We should be updatingmax_table_size
as part of the resize behaviour.max_table_size
issue above, the current behaviour ofHPAX.resize/2
is insufficient on the encoder side. If the max size has decreased, we must send a dynamic table size update to the decoder.There are four existing calls to
HPAX.resize/2
across all of HPAX's hex dependents:Proposed Solution
Based on the above, I propose that we should:
Move the existing
HPAX.Table.resize/2
function to a privateHPAX.Table.evict_to_max_size/2
since it is used in table addition (its current behaviour is correct for this purpose).Create a new
HPAX.Table.resize/2
implementation that callsHPAX.Table.evict_to_max_size/2
and also sets the value ofmax_table_size
as indicated. This will correctly implement the behaviour required when it is called on the decoder (both explicitly by the application upon receipt of settings frame asks, and also as part of receiving a dynamic table size update).Add logic to
HPAX.Table.resize/2
to set a flag on the table whenever the table max size is adjusted downward. This flag should be checked when encoding, and if set the encoding should have a dynamic table size update prepended (and the flag subsequently cleared). There is no need to discriminate between encoder and decoder users here, since decoders will never be encoding (and so the flag is vestigial in this case).I'm happy to undertake this work, but given its subtlety I think it's good to have multiple people think through it and ensure it's a sensible solution.
[^1]: Of note, this process was not specified as part of the original RFC 7540; details were only added in RFC 9113 (see Appendix B).