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

Support for permessage-deflate [✅/awaiting_dependency_merge] #2

Open
nox opened this issue Mar 22, 2017 · 26 comments · Fixed by #328
Open

Support for permessage-deflate [✅/awaiting_dependency_merge] #2

nox opened this issue Mar 22, 2017 · 26 comments · Fixed by #328

Comments

@nox
Copy link

nox commented Mar 22, 2017

This is a mandatory part of the spec, and is thus needed to be Web-compatible.

@agalakhov
Copy link
Member

agalakhov commented Mar 22, 2017

Thanks, it will be done. The only reason not having it already is, we don't use it ourselves, and the library was written for our production use.

Note for myself: it is RFC 7692.

@nox
Copy link
Author

nox commented Mar 22, 2017

I think I brainfarted when I thought it is mandatory.

@agalakhov
Copy link
Member

At least it is required by Autobahn, that's enough for me :)

Currently tungstenite passes all the Autobahn tests with only two of them as "non-strict", and my plan is to achieve 100% strict (I know what exactly fails). For comparison, ws-rs has 11 non-stricts and others are even worse. So the support for RFC 7692 is needed at least to have 100% green lines here. :)

@nox
Copy link
Author

nox commented Mar 23, 2017

It is actually mandatory for a user agent, see step 9 of https://fetch.spec.whatwg.org/#concept-websocket-establish.

@dariost
Copy link

dariost commented Mar 28, 2017

As a suggestion I'd say to implement this using something like flate2 and not libz-sys in order to avoid external dependencies on system libraries

@Fedcomp
Copy link

Fedcomp commented Aug 4, 2018

Is it still planned? ws-rs support permessage-deflate but does not support async :(

@daniel-abramov
Copy link
Member

Yes, we still plan to work on it, but unfortunately we did not have time to work on it yet as it was lower in priorities than other things which we fixed / improved since then.

psmit pushed a commit to psmit/tungstenite-rs that referenced this issue Nov 14, 2019
psmit pushed a commit to psmit/tungstenite-rs that referenced this issue Nov 14, 2019
@AlexDaniel
Copy link

AlexDaniel commented Nov 24, 2020

Any updates on this? This is the current state of permessage-deflate in rust's websocket libraries:

@tinaun
Copy link

tinaun commented Nov 24, 2020

#144

@daniel-abramov
Copy link
Member

Someone needs to update the #144 to the newest state (resolve the conflicts and fix some remarks in code), once it's done and clean, we could merge it. Unfortunately we don't plan to make changes at this particular place in the nearest future, but if someone makes a PR and/or updates the existing #144 which seems to be [almost] ready (?), we would be glad to review and merge it.

@Fedcomp
Copy link

Fedcomp commented Dec 1, 2020

We should probably mention @SirCipher to update pull request :)

@SirCipher
Copy link

@application-developer-DA, @Fedcomp, sorry for the delay, we've been updating to Tokio 0.3. The work is nearly done and I'll update the PR soon.

@rymnc
Copy link

rymnc commented Jan 12, 2021

Do you have an update?
Thanks!

@SirCipher
Copy link

SirCipher commented Jan 13, 2021

@rymnc, the PR is still awaiting a review at the moment. I've got a local branch that's updated to the most recent version for both this repository and tokio-tungstenite that I'll commit soon too

@rymnc
Copy link

rymnc commented Jan 13, 2021

Okay, np
thanks for the reply

@ignatov
Copy link

ignatov commented Apr 9, 2021

Hi, do you have any news about the permessage-deflate support?

@daniel-abramov
Copy link
Member

The corresponding PR was closed as the author did not have to time to update it / address the remarks, so he decided to close the PR. The new PRs are welcome ;)

@kazk
Copy link
Contributor

kazk commented Sep 10, 2021

I'm currently working on this. So far, I've got it to pass some autobahn (no parameter support, so client passes 12, server passes both 12 and 13) by layering compression/decompression on top of the existing flow, so it doesn't need as much code, and it's clearly additive. I don't think there's any major breaking changes.

$ git diff --cached --stat src/
 src/error.rs                |  14 ++++-
 src/extensions/deflate.rs   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/extensions/mod.rs       |  89 ++++++++++++++++++++++++++++
 src/handshake/client.rs     |  87 +++++++++++++++++++++++----
 src/handshake/server.rs     |  28 ++++++++-
 src/lib.rs                  |   1 +
 src/protocol/frame/frame.rs |  11 ++++
 src/protocol/mod.rs         | 137 +++++++++++++++++++++++++++++++++++++++----
 8 files changed, 558 insertions(+), 26 deletions(-)

For now, I'm not planning to implement server_max_window_bits/client_max_window_bits support because that requires flate2/zlib feature, and I don't need them, but it should be easy to add them later with a separate feature flag.

Later, we should be able to support other compressions like snappy too.

@kazk
Copy link
Contributor

kazk commented Sep 13, 2021

Opened #235. Any feedback is appreciated.

@TheDan64
Copy link

TheDan64 commented Dec 1, 2022

@kazk Is there any assistance needed to help get this PR across the finish line?

@kazk
Copy link
Contributor

kazk commented Dec 2, 2022

@TheDan64
It's blocked by hyperium/headers#88. More specifically hyperium/headers#88 (comment). Maybe providing some feedback on that will be helpful to get that merged.

Once that's merged, I'll open a new PR from a branch using it, which is a rebased version of #235 with header parsing extracted and with better error handling.

@TheDan64
Copy link

TheDan64 commented Dec 20, 2022

Tried out your rebased branch for our use case and had no issues - appears to be working well. Hope to see this upstreamed sooner than later so that we can use it in production & with the other tungstenite sister libraries

@daniel-abramov daniel-abramov changed the title Support for permessage-deflate Support for permessage-deflate [✅/awaiting_dependency_merge] Mar 22, 2023
@daniel-abramov
Copy link
Member

The implementation by @kazk resides in https://github.com/snapview/tungstenite-rs/tree/permessage-deflate and has been temporarily reverted from a master branch, see the discussion in the PR for more details.

@nakedible-p
Copy link

FWIW, I have rebased branches of all the relevant functions all the way to axum-tungstenite (alternative to axum::extract::ws) to use the permessage-deflate. Tested to work fine.

My specific use case needs high performance, and configurable window bits, so there is some hardcoding of window bits as I didn't want to implement full window bits handling yet. I can implement full window bits handling... but I'd want the permessage-deflate implementation by @kazk merged first.

hyperium/headers@master...nakedible-p:hyperium-headers:deflate-release
master...nakedible-p:snapview-tungstenite-rs:deflate-release
snapview/tokio-tungstenite@master...nakedible-p:snapview-tokio-tungstenite:deflate-release
davidpdrsn/axum-tungstenite@main...nakedible-p:davidpdrsn-axum-tungstenite:deflate-release

@SvizelPritula
Copy link

Would it maybe be a good idea to implement the parsing of the Sec-Websocket-Extensions header into this crate? The PR agains headers has shown no progress for over ten months. If the PR ever gets merged, it should be pretty simple to switch over to using that version.

@kaszperro
Copy link

Could we make it without headers PR? Such a great feature is blocked because of external dependency for such a long time.

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