Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP/2 apply SETTINGS configuration on reply #9801

Open
sbordet opened this issue May 26, 2023 · 3 comments
Open

HTTP/2 apply SETTINGS configuration on reply #9801

sbordet opened this issue May 26, 2023 · 3 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented May 26, 2023

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 sbordet added the Bug For general bugs on Jetty side label May 26, 2023
@sbordet
Copy link
Contributor Author

sbordet commented May 26, 2023

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
Copy link
Contributor Author

sbordet commented May 26, 2023

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.

Copy link

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.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label May 26, 2024
@sbordet sbordet removed the Stale For auto-closed stale issues and pull requests label May 27, 2024
@gregw gregw removed this from Jetty 12.1.0 Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
Status: No status
Development

No branches or pull requests

1 participant