Add permessage-deflate support

[kazk]

Supersedes #235.

• Adds optional de/compression logic on top of the existing flow without any major breaking changes.
• permessage-deflate is not enabled by default and requires a deflate feature.
• client_max_window_bits and server_max_window_bits are not supported yet because these require flate2/zlib which is not pure Rust.
• Passes Autobahn except for 13.3 to 13.6. These require supporting max window bits.
• Correct extension negotiation. The server declines invalid offers, and the client errors on invalid response to terminate the connection.
• Sec-WebSocket-Extensions header is parsed by headers
  • Waiting for Add SecWebsocketExtensions hyperium/headers#88
• Add permessage-deflate support tokio-tungstenite#251
• Clean warp integration: Add WebSocket permessage-deflate extension support seanmonstar/warp#1016

I might need to adjust some code depending on hyperium/headers#88, but this should be ready for review.

---

Closes #2.

[daniel-abramov]

I did not check it rigorously, but it looks like a sane implementation, thanks for the amount of work that has been put into it! It also looks like it worked pretty well (based on the comments to the previous iteration of the PR).

It's a significant change, so I'll also additionally ask @agalakhov to approve it so that we can safely merge it!

[617a7aa]

Any review status on this?

[daniel-abramov]

There have not been any comments, so I assume that everyone is okay with the changes, so let's give it a go!

Thanks again for the large amount of work that has been put into that.

[nakedible-p]

I was fighting for quite a while trying to get this work and it didn't... until I finally realized that it was merged in yesterday! Long time in the making, glad to finally have it implemented.

[kazk]

@daniel-abramov As I mentioned in the PR, this depends on hyperium/headers#88.

[daniel-abramov]

@daniel-abramov As I mentioned in the PR, this depends on hyperium/headers#88.

Oh, yeah, thanks for reminding me, I've forgotten about that! Though maybe we could specify it in the README if we want to publish it as a separate feature to let people decide. Hopefully, the hyperium/headers#88 gets reviewed and merged. It seems like maintainers did not have time to address it recently or something like that.

It seems like people really want to use permessage-deflate with WebSockets.

[kazk]

You won't be able to publish tungstenite for any other updates/fixes because of the git dependencies. If users don't mind having git dependencies in their apps, they could've used my fork until this was merged.

It seems like people really want to use permessage-deflate with WebSockets.

Yeah, it can be significant depending on your messages.

It also looks like it worked pretty well (based on the comments to the previous iteration of the PR).

I've been using the original implementation (#235) in production for almost 2 years without any issues, but the new implementation in this PR is not battle tested. It should be equivalent and have better error handling.

Anyway, I'm just surprised this was merged.

Are you the only active maintainer?

[daniel-abramov]

You won't be able to publish tungstenite for any other updates/fixes because of the git dependencies. If users don't mind having git dependencies in their apps, they could've used my fork until this was merged.

Yeah, that's unfortunate. I may need to move it to a branch (which essentially would defeat the whole purpose of merging I think). It felt like if we don't merge it, it would stagnate and eventually will get obsolete (conflicts with the master branch etc), but at the same time, I'd forgotten about the git dependencies.

I've been using the original implementation (#235) in production for almost 2 years without any issues, but the new implementation in this PR is not battle tested. It should be equivalent and have better error handling.

That does not sound too bad.

Anyway, I'm just surprised this was merged.
Are you the only active maintainer?

Well, sort of. I'm not the only maintainer, but the only one who has been more or less active in the past years.

UPD: Reverted the change, and moved the implementation to the branch until the dependency is updated.

[nakedible-p]

This is incorrect. Server may include "server_max_window_bits" without the client asking for it for the explicit reason that it informs the window size for compression that the server uses. There is no harm for the client to ignore that value, and use a full sized window for decompression - and since our window is 15 bits by default now that it isn't configurable, any valid value from server is fine for the client.

The correct behaviour is to have the normal duplicate checking for this response header, but accept any valid value (or any value below the window size we use, but flate2 doesn't expose that so we will just need to assume it's 15 bits).

[zy010101]

Thanks for your comment.

[nakedible-p]

@kazk The pull is merged, and the work is stashed in a branch and now there's no open pull for this... but added one review comment that has an impact on the interoperability of the client.

[MrFoxPro]

@kazk hyperium/headers#88 (comment)

[bouk]

So it seems someone could rebase this PR on master, but putting the sec_websocket_extensions.rs module in this repository directly, since it won't be upstreamed.

[SvizelPritula]

I'm working on integrating sec_websocket_extensions.rs into this crate, will post a (draft) PR soon.