jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.85k stars 1.91k forks source link

QPack encoder must not send any encoder instructions when SETTINGS_QPACK_MAX_TABLE_CAPACITY is 0 #12341

Closed AlekseiEfimov closed 1 week ago

AlekseiEfimov commented 2 weeks ago

Jetty version(s) Jetty-12

Jetty Environment core

Java version/vendor N/A since the report is based on source code and RFC 9204

OS type/version N/A since the report is based on source code and RFC 9204

Description According to RFC 9204, the QPack encoder should not send instructions on the encoder stream when the maximum dynamic table capacity value is 0. This value is determined by the SETTINGS_QPACK_MAX_TABLE_CAPACITY sent by the peer's decoder.

Here is the RFC 9204 section 3.2.3 quote:

When the maximum table capacity is zero, the encoder
 MUST NOT insert entries into the dynamic table
 and MUST NOT send any encoder instructions on the encoder stream.

The encoder is issuing the "Set Dynamic Table Capacity" with capacity set to the minimum of the received and configured max capacity values:

encoder.setTableCapacity(Math.min(maxTableCapacity, configuration.getMaxEncoderTableCapacity()));

If the resulting value is 0 - the encoder instruction will be issued unconditionally to the max capacity value. It is correct for positive values, but for 0, it is not - the instruction must not be sent according to the RFC statement above.

How to reproduce? Use any HTTP/3 client to connect to Jetty server. Client-side steps: a) send a settings frame with SETTINGS_QPACK_MAX_TABLE_CAPACITY set to 0 (which is the default value). b) verify that no encoder instructions (including set dynamic table capacity) are received on the client's encoder stream after that.

lachlan-roberts commented 1 week ago

I think that line in the RFC is a bit vague. But if you take it literally, then since the table capacity starts at zero, you can never change it to a non-zero value because you cannot send the instruction to do so.

So I don't agree that sending this instruction violates the RFC, I think this should be interpreted as "you cannot use any instruction to alter to contents of the dynamic table if the capacity is zero".

lachlan-roberts commented 1 week ago

But since this instruction is redundant we can skip it anyway.

I will open a PR to only send the instruction if it actually changed the table capacity. And this should fix your concern with it.