tungstenite-rs
tungstenite-rs copied to clipboard
Add `permessage-deflate` support
- Adds de/compression logic on top of the existing flow, requiring minimal changes. Shouldn't have any major breaking changes.
-
client_max_window_bits
/server_max_window_bits
are not supported at the moment because that requiresflate2/zlib
feature. We can enable them with a feature flag later if there's a demand. - Passes autobahn except for 13.3 to 13.6, that requires supporting max window bits
- Can be used from crates like
warp
andaxum
.-
tokio-tungstenite
: https://github.com/snapview/tokio-tungstenite/pull/190 -
warp
: https://github.com/seanmonstar/warp/compare/master...kazk:permessage-deflate - Example using
warp
with compression: https://github.com/qualified/lsp-ws-proxy/commit/89fce502cf792cfa89e814865bbe170666499c0d
-
This works for my use case, but I'd like some feedback from maintainers and users before polishing this to be merged.
- Does it make sense to think about supporting more PMCEs (e.g.,
permessage-bzip2
andpermessage-snappy
)? These are not standardized as far as I know, and browsers don't support them. - ~~I think including
flate2
without feature flag is fine (not large, defaults to pure Rust). What do you think? If there's a demand, we can add a flag forflate2/zlib
and support max window bits.~~ - Should the default
WebSocketConfig
enable compression support?
- [x]
deflate
feature - [x] Parsing
Sec-WebSocket-Extensions
more strictly withheaders
(https://github.com/kazk/tungstenite-rs/tree/permessage-deflate-rebased) - [ ] Error on invalid negotiation response
- [ ] Error handling
- [ ]
deflate-zlib
feature for max window bits
Closes #2
@application-developer-DA
Thanks for reviewing! I'll start answering them soon. I opened this PR as a draft since it's not done (especially the client side like error handling, as you noticed), but wanted early feedback.
@kazk Any chance this is still being worked on?
I was planning to continue after getting https://github.com/hyperium/headers/pull/88 merged (see https://github.com/snapview/tungstenite-rs/pull/235#discussion_r742305530), but forgot. I've been using this branch with a patched warp
in production.
Will try to take a look this week after work.
I think the following needs to be done to get this merged:
- Improve parsing
Sec-WebSocket-Extensions
. I did in https://github.com/hyperium/headers/pull/88, but we can include that here for now if necessary. - Validate negotiation response
- Error handling in general
Nice to have:
-
deflate-zlib
feature to support max window bits
For now, I've rebased and cleaned up the commits. https://github.com/kazk/tungstenite-rs/tree/permessage-deflate-rebased
I might have some time over the weekend to continue.
Improve parsing Sec-WebSocket-Extensions. I did in https://github.com/hyperium/headers/pull/88, but we can include that here for now if necessary.
NB: Should we probably wait a bit for a merge of the PR in headers
so that we can simplify the implementation in the tungstenite
?
Yeah, I'd like to keep that in headers
.
I'll update this PR to use headers
(using my fork for now), and make sure it's useful for tungstenite
. After making any necessary changes, I'll ask them to review that PR again and try to get that merged.
I've updated to use headers
. https://github.com/kazk/tungstenite-rs/commit/2e404a447c94eb05f3ee4059b7094bad06e20d1f
I'll try to spend some time during the week to finish it and open a new PR.
@application-developer-DA
The autobahn results don't match the expected in https://github.com/snapview/tungstenite-rs/tree/master/ for both server and client. ~~Is this a regression? I can open a separate PR to update if it just needs to be updated.~~
These are tests for Clean close with normal or echoed code
. I guess tungstenite
used to close with 1000
, but now closes with echoed code. #246?
Opened #271
diff expected master
:
"7.7.10": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 3000,
"reportfile": "tungstenite_case_7_7_10.json"
},
"7.7.11": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 3999,
"reportfile": "tungstenite_case_7_7_11.json"
},
"7.7.12": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 4000,
"reportfile": "tungstenite_case_7_7_12.json"
},
"7.7.13": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 4999,
"reportfile": "tungstenite_case_7_7_13.json"
},
"7.7.2": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1001,
"reportfile": "tungstenite_case_7_7_2.json"
},
"7.7.3": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1002,
"reportfile": "tungstenite_case_7_7_3.json"
},
"7.7.4": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1003,
"reportfile": "tungstenite_case_7_7_4.json"
},
"7.7.5": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1007,
"reportfile": "tungstenite_case_7_7_5.json"
},
"7.7.6": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1008,
"reportfile": "tungstenite_case_7_7_6.json"
},
"7.7.7": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1009,
"reportfile": "tungstenite_case_7_7_7.json"
},
"7.7.8": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1010,
"reportfile": "tungstenite_case_7_7_8.json"
},
"7.7.9": {
"behavior": "OK",
"behaviorClose": "OK",
- "remoteCloseCode": 1000,
+ "remoteCloseCode": 1011,
"reportfile": "tungstenite_case_7_7_9.json"
}
@kazk thanks for the investigation! The corresponding PRs have been merged.