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

Ignore client_max_window_bits with missing header #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kuroneer
Copy link

@Kuroneer Kuroneer commented Oct 5, 2021

When the client does not provide the "client_max_window_bits" option,
the server must reserve the max size.

Currently, if the server provides the option client_max_window_bits with something less than 15 and a client does not include the option and uses more than the unknown server value, zlib will fail to inflate the payload.

I'll leave this in draft until I'm able to test it.

Are automatic tests for this functionality expected in cowboy?

@Kuroneer
Copy link
Author

Kuroneer commented Oct 5, 2021

The only error in the gh actions is
Error: Couldn't produce an instance that satisfies all strict constraints from (cow_http_hd:content_range/0) after 50 tries.

I have tested it successfully both manually with a python client and within cowboy test suites.

@Kuroneer Kuroneer marked this pull request as ready for review October 5, 2021 18:10
@essen
Copy link
Member

essen commented Oct 6, 2021

You can run make ct-ws_autobahn in Cowboy to run full tests related to the protocol. make ct-ws for the other ones (this is likely where one test would sit for this).

I don't think this is the right fix what it's worth. This setting on the server indicates that the server doesn't want to accept windows larger than a certain size. I believe the correct behavior when that happens is to return ignore instead.

When the client does not provide the "client_max_window_bits" option,
the server must be able to handle a sliding window of up to 32,768 bytes.

In case that the window has a lower limit set by options, compression must
be disabled.

Co-authored-by: Ignacio Martínez <[email protected]>
@Kuroneer Kuroneer force-pushed the client-less-window-bits-without-header branch from 5990041 to a6fd4f1 Compare October 7, 2021 12:18
@Kuroneer
Copy link
Author

Kuroneer commented Oct 7, 2021

(force pushed to remove the previous commit from the history and fix the message)

@Kuroneer
Copy link
Author

@essen Is there anythin else you'd like to see changed?

@essen
Copy link
Member

essen commented Oct 14, 2021

I just need to find the time to review and test.

@essen essen added this to the Gun 2.0 milestone Mar 3, 2022
@essen essen removed this from the Gun 2.0 milestone Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants