python-hyper / h2

Pure-Python HTTP/2 protocol implementation
https://python-hyper.org/
MIT License
968 stars 157 forks source link

Updating `H2Connection.local_settings.max_frame_size` does not update `H2Connection.max_inbound_frame_size`. #1288

Closed sujaldev closed 6 days ago

sujaldev commented 6 days ago

I am using the following code to update the MAX_FRAME_SIZE setting:

conn = H2Connection(config)
conn.local_settings.max_frame_size = 2**18
conn.local_settings.acknowledge()

but this causes a FRAME_SIZE_ERROR, so I also need to append:

conn.max_inbound_frame_size = 2**18

Shouldn't max_inbound_frame_size be implemented with a setter/getter instead of just copying from Settings at __init__?

Kriechi commented 6 days ago

Do you actually call conn.local_settings.acknowledge() as shown in your code snippet?

I believe this happens automatically once the other side of the connection acknowledges that this setting is valid, see here: https://github.com/python-hyper/h2/blob/eaa1489e57531a555a40731e0bc8b3b6e9cdffc3/src/h2/connection.py#L1704

which then updates max_inbound_frame_size here: https://github.com/python-hyper/h2/blob/eaa1489e57531a555a40731e0bc8b3b6e9cdffc3/src/h2/connection.py#L1938

In your case, you are forcing a max frame size without waiting for the other side to ACK it - which might lead to connection or protocol errors.

sujaldev commented 6 days ago

Ah, I didn't realize this is called automatically. My intention, however, is to override the default settings sent in the initial SETTINGS frame, which does not contain the overridden settings if I do not call acknowledge(). I'd also like to avoid sending an additional SETTINGS frame via update_settings() as it seems unnecessary and would require modifying tests. I suppose I'll do as suggested in #1042 to override the defaults.

Thanks.