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

Clients should reject any attempt to change the SETTINGS_ENABLE_PUSH setting to a non-zero value #316

Open
alexwlchan opened this issue Sep 19, 2016 · 1 comment

Comments

@alexwlchan
Copy link
Contributor

alexwlchan commented Sep 19, 2016

RFC 7540 section 8.2:

Clients MUST reject any attempt to change the SETTINGS_ENABLE_PUSH setting to a value other than 0 by treating the message as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.

If I’ve read this correctly, “attempt to change” means a SettingsFrame, yes?
If so, we don’t do this yet – indeed, there doesn’t seem to be a way for the h2.settings.Settings object to know if it’s for a client or server.

I’m not sure exactly when the Settings object gets configured – the user can specify the initial_values parameter, so presumably a client could have ENABLE_PUSH = 1 set by this way. The validation can’t occur on the Settings object, but we actually receive the frame, in H2Connection._receive_settings_frame().

That seems sub-optimal – it would be good to keep settings-specific logic in settings.py.

Perhaps we add a validate_received_setting() to mirror _validate_setting(), which is defined in settings.py but called in _receive_settings_frame(). This contains any additional checks for settings that come in via a SettingsFrame.

@alexwlchan alexwlchan added the Bug label Sep 19, 2016
@Lukasa
Copy link
Member

Lukasa commented Sep 20, 2016

Attempt to change does mean a SettingsFrame, yes. However, it seems like you're getting a bit turned around about setting stuff.

Yes, a client can have ENABLE_PUSH=1 set by the initial_values parameter, but that can only happen for the settings it offers to its peer. Remember that, like with flow control windows, there are always two collections of active settings: those sent by the client to the server, and those sent by the server to the client.

However, all of this is easier than you think because the H2Settings object already gets told whether it's for a client or not. All we need to do to add extra validation is to persist that value on the object, and then extend _validate_setting to take a client parameter that determines whether the entity setting the value is a client or not. This is probably pretty easy, tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants