Add `SecWebsocketExtensions`
I'm working on https://github.com/seanmonstar/warp/issues/770 and noticed this is missing.
Does this type by itself provide any use? I imagine you need to do more with it than simply materialize an opaque value from the map.
This header is used to negotiate the usage of WebSocket extensions (extension itself and its parameters). The server needs to be able to:
- Iterate offer(s) (offers are separated by commas)
- Find a first valid offer for each extension by checking parameters in the offer (parameters are semicolon separated, the first element is the name of the extension)
- Include the agreed parameters for each extension in the response header
The header value depends on the WebSocket extension. As far as I know, the only standardized extension at the moment is permessage-deflate and it's used like this:
# Just the name of the extension (default parameters)
Sec-WebSocket-Extensions: permessage-deflate
# With parameters
Sec-WebSocket-Extensions: permessage-deflate; client_no_context_takeover; client_max_window_bits=10;
# With multiple sets of parameters separated by comma (the first valid set is accepted)
Sec-WebSocket-Extensions:
permessage-deflate; client_no_context_takeover; server_max_window_bits=10,
permessage-deflate; client_no_context_takeover; client_max_window_bits,
permessage-deflate; client_no_context_takeover
In tungstenite (doesn't use this crate at the moment), I'm doing something naive like this:
// Parse `Sec-WebSocket-Extensions` offer/response. Doesn't support quoted values.
fn parse_header(exts: &str) -> Vec<(Cow<'_, str>, Vec<Param<'_>>)> {
let mut collected = Vec::new();
for ext in exts.split(',') {
let mut parts = ext.split(';');
if let Some(name) = parts.next().map(str::trim) {
let mut params = Vec::new();
for p in parts {
let mut kv = p.splitn(2, '=');
if let Some(key) = kv.next().map(str::trim) {
let param = if let Some(value) = kv.next().map(str::trim) {
Param::new(key).with_value(value)
} else {
Param::new(key)
};
params.push(param);
}
}
collected.push((Cow::from(name), params));
}
}
collected
}
#[derive(Clone, Debug, Eq, PartialEq)]
struct Param<'a> {
name: Cow<'a, str>,
value: Option<Cow<'a, str>>,
}
#[test]
fn test_parse_extensions_header() {
let extensions = "permessage-deflate; client_max_window_bits; server_max_window_bits=10, permessage-deflate; client_max_window_bits";
assert_eq!(
parse_header(extensions),
vec![
(
Cow::from("permessage-deflate"),
vec![
Param::new("client_max_window_bits"),
Param::new("server_max_window_bits").with_value("10")
]
),
(Cow::from("permessage-deflate"), vec![Param::new("client_max_window_bits")])
]
);
}
So, maybe pub struct SecWebsocketExtensions(FlatCsv<Comma>); with some helper methods (using FlatCsv<SemiColon>)?
I'd like to take over pushing permessage-deflate through the WebSockets stack.
Which review points have to be addressed in order to get this PR merged?
@ilammy I was waiting on a response from @seanmonstar for https://github.com/hyperium/headers/pull/88#discussion_r711390478.
Once this is merged, I already have a branch using it, which is a rebased version of my original tungstenite-rs PR.
@kazk @seanmonstar I'm humble ask to return to review this PR as dependent functionality in tungstenite-rs extremely important for my team, but in same time we have strict policy don't use libraries from branches, so we have to wait for finalized published version.
@gallowsmaker I can't do anything for this PR, but I'll open a new PR for tungstenite-rs so we can start reviewing the new version while we wait for this. It will also clarify what needs to be done. Looks like I need to rebase again and there are some conflicts, but I should be able to open it soon.
I've been using the original version in production without any issues, but I'd love to get these merged and move on.
@seanmonstar just checking if you're still able to review this?
It would be super nice to get this merged - any hope for this to progress?
I'm waiting
@seanmonstar @kazk bump. What's the blocker here? What help do you need?
any update ? I'm waiting too ....
I'm waiting too
This PR is highly needed feature. It is blocker for us for using permessage-deflate. What can I do, to speedup merging of this?
@seanmonstar are there any plans for this to be merged, or do you require any changes to be made? Getting this merged unblocks a PR in tungstenite that provides permessage-deflate support for websockets.
@seanmonstar What would it take to get this merged? I'm willing to sponsor this work
I know there's a lot of interest here. I've describe this a few times in chat, but never here. So, let me try:
- This crate tries to expose built-in header types which expose as little internal details as possible. The reason for that is to try to limit mistakes that occurred when this was
hyper::headerin older versions, and changes to the exposed internals would mean breaking changes. - It's not clear whether
SecWebsocketExtensionsshould be optimized for parsing, or for mutating.- That changes whether it should internally hold onto a flattened string, and parse on the fly of
fn iter() -> impl Iterator, or if it should store everything parsed at construction in a list. - Unfortunately, that internal decision seems like it will leak, since the iterator would either provide
WebsocketExtension<'a>s referring to the flat string, or&WebsocketExtensionif an owned item inside a list. - Most asking for this feature are more interested in it just working (understandably), than the maintenance details afterwards.
- That changes whether it should internally hold onto a flattened string, and parse on the fly of
-
This doesn't need to block people. This is the most important thing that is missed. Since the crate is based on a
trait Header, any other code is able to declare its own typed header andimpl Header for MySecWebSocketExtensions.- That code can start in tungstenite, or wherever else that's wanting this, and the design and performance of the type can be tested and tweaked there.
- Once those questions are answered and tested, we can merge something here for longer term maintenance.