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.83k stars 1.91k forks source link

HTTP/2 apply SETTINGS configuration on reply #9801

Open sbordet opened 1 year ago

sbordet commented 1 year ago

Jetty version(s) 10+

Description Currently, the SETTINGS configuration is applied locally before receiving the SETTINGS reply from the other peer.

This may cause races between the local peer that already has the new configuration, and the remote peer which is not aware of the new configuration.

sbordet commented 1 year ago

The implementation needs to coordinate with the writes, so would be best to queue into the flusher a fake frame that updates the configuration and sends the SETTINGS reply.

In this way, there is a window where both sides work on default settings, but it is guaranteed that when a peer receives a SETTINGS reply, then subsequent frames have seen the new configuration.

sbordet commented 1 year ago

Error scenario 1:

server configured with maxFrameSize=100

client sends empty SETTINGS (all values at default) client sends HEADERS whose frame size is 200

server receives SETTINGS server receives HEADERS => error: frame size too large

Error scenario 2:

server configured with maxTableCapacity=0

client sends empty SETTINGS (all values at default) client sends HEADERS with table update instruction to 4096 (the default)

server receives SETTINGS server receives HEADERS => error decoding as the table update instruction value is greater than allowed


Fixed Scenario 1

server prepares configuration with maxFrameSize=100, but does not apply it yet server sends SETTINGS(maxFrameSize=100)

Sub-Scenario 1a

client receives SETTINGS(maxFrameSize=100) client applies configuration client sends SETTINGS(reply) client sends empty SETTINGS (all values at default) client sends HEADERS whose frame size is 100 as per configuration

server receives SETTINGS(reply) server updates configuration with maxFrameSize=100 server receives SETTINGS server receives HEADERS whose frame size is 100 => all good

Sub-Scenario 1b

client sends empty SETTINGS (all values at default) client sends HEADERS whose frame size is 200 (default is still 16384). client receives SETTINGS(maxFrameSize=100) client applies configuration client sends SETTINGS(reply)

server receives empty SETTINGS server sends SETTINGS(reply) server receives HEADERS whose frame size is 200, the configuration is still at default of 16384 => all good server receives SETTINGS(reply) server applies configuration


The solution applies the configuration only when receiving a SETTINGS(reply). This leaves a window where frames can be exchanged with default values; after the SETTINGS reply is processed the new configuration is in place with the updated values. This is ok.

However, there still is a race between the network reader thread writing the configuration and the network writer thread reading the configuration. For example, the network reader thread writes the configuration with a new value while the network writer thread reads the configuration and the new value; then the network reader thread enqueues the SETTINGS(reply). The receiving side will receive a frame with the new value, which may be invalid because the configuration has not been applied yet, and then receive the SETTINGS(reply) that will apply the configuration.

To avoid this problem, the network reader thread could enqueue a synthetic frame that writes the configuration, so that only the network writer thread reads and writes the configuration, in the order the frames are enqueued.

github-actions[bot] commented 4 months ago

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.