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 WebSocket upgrade #347

Closed
ubnt-intrepid opened this issue Feb 23, 2019 · 10 comments · Fixed by #655
Closed

Support WebSocket upgrade #347

ubnt-intrepid opened this issue Feb 23, 2019 · 10 comments · Fixed by #655

Comments

@ubnt-intrepid
Copy link

RFC 8441 specifies the upgrade mechanism to WebSocket over HTTP/2. Since it includes several extensions to the HTTP/2 protocol, the server API might need to support the configuration of them.

@carllerche
Copy link
Collaborator

Would you be able to provide a summary of the ways in which h2 needs to change to support this use case?

@ubnt-intrepid
Copy link
Author

Here is a summary of RFC:

  • Adds a new SETTINGS parameter SETTINGS_ENABLE_CONNECT_PROTOCOL (=0x8) is added[^1]. The server sends this parameter with the value 1 at the beginning of connection.
  • The handshake request uses CONNECT method instead of GET [^2]. The header fields used for handshaking are similar to the current WebSocket spec, but several pseudo-header been added or removed.
  • After the request is upgraded, the payloads are send and received via DATA frame.

[^1] : https://tools.ietf.org/html/rfc8441#section-3
[^2] : https://tools.ietf.org/html/rfc8441#section-4

@carllerche
Copy link
Collaborator

Seems plausible. I wonder if we need to add support for that specific settings option or we can extend h2 to allow for configurable settings.

@bnoordhuis
Copy link

I've been looking into this. Adding SETTINGS_ENABLE_CONNECT_PROTOCOL is straightforward but I'm stuck on exposing the :protocol pseudo-header.

The client needs to send it, the server needs to receive it, but I can't pass it through http::Request because it's not a legal traditional HTTP header. Suggestions on how to proceed welcome.

@seanmonstar
Copy link
Member

Hm, ya... The first thing that comes to my mind is to store some h2::Something type in the Request::extensions() indicating a :protocol pseudo header. Does that work cleanly? Should it?

@bnoordhuis
Copy link

I had the same thought and have a local prototype that adds a pub fn websocket(req: &mut Request<()>) { /* ... */ } that does that (but probably needs a better name.)

It feels kind of icky for reasons I can't quite articulate though. :-)

@Icelk
Copy link

Icelk commented Aug 23, 2022

Is it feasible to add the pseudo header information to the RecvStream?

@Icelk
Copy link

Icelk commented Aug 29, 2022

Is there any way I can help?
I would appreciate if we can implement this, so I can use WebSockets on HTTP/2 :)

@cloneable
Copy link
Contributor

I believe this was largely done by @nox in #565 a year ago. Anything missing? It looks like h2::ext::Protocol is not added as extension to the Request, at least I don't see it even though tracing shows that h2 received headers containing the :protocol pseudo header (set to b"websocket" in my case).

The server tests don't seem to cover this.
https://github.com/hyperium/h2/blob/07d20b19abfd3bf57bd80976089e3c24a3166bca/tests/h2-tests/tests/server.rs
I was hoping for an example that shows how one can check for "the-bread-protocol" on the server side.

@cloneable
Copy link
Contributor

I added the following code to server::Peer::convert_poll_message() and that exposes the protocol as Request extension. This works for me. Not sure if this is the right place to do this, though.

if let Some(p) = pseudo.protocol {
    if let Some(exts) = b.extensions_mut() {
        exts.insert(p);
    }
}

h2/src/server.rs

Lines 1427 to 1535 in 07d20b1

fn convert_poll_message(
pseudo: Pseudo,
fields: HeaderMap,
stream_id: StreamId,
) -> Result<Self::Poll, Error> {
use http::{uri, Version};
let mut b = Request::builder();
macro_rules! malformed {
($($arg:tt)*) => {{
tracing::debug!($($arg)*);
return Err(Error::library_reset(stream_id, Reason::PROTOCOL_ERROR));
}}
}
b = b.version(Version::HTTP_2);
let is_connect;
if let Some(method) = pseudo.method {
is_connect = method == Method::CONNECT;
b = b.method(method);
} else {
malformed!("malformed headers: missing method");
}
let has_protocol = pseudo.protocol.is_some();
if !is_connect && has_protocol {
malformed!("malformed headers: :protocol on non-CONNECT request");
}
if pseudo.status.is_some() {
malformed!("malformed headers: :status field on request");
}
// Convert the URI
let mut parts = uri::Parts::default();
// A request translated from HTTP/1 must not include the :authority
// header
if let Some(authority) = pseudo.authority {
let maybe_authority = uri::Authority::from_maybe_shared(authority.clone().into_inner());
parts.authority = Some(maybe_authority.or_else(|why| {
malformed!(
"malformed headers: malformed authority ({:?}): {}",
authority,
why,
)
})?);
}
// A :scheme is required, except CONNECT.
if let Some(scheme) = pseudo.scheme {
if is_connect && !has_protocol {
malformed!(":scheme in CONNECT");
}
let maybe_scheme = scheme.parse();
let scheme = maybe_scheme.or_else(|why| {
malformed!(
"malformed headers: malformed scheme ({:?}): {}",
scheme,
why,
)
})?;
// It's not possible to build an `Uri` from a scheme and path. So,
// after validating is was a valid scheme, we just have to drop it
// if there isn't an :authority.
if parts.authority.is_some() {
parts.scheme = Some(scheme);
}
} else if !is_connect || has_protocol {
malformed!("malformed headers: missing scheme");
}
if let Some(path) = pseudo.path {
if is_connect && !has_protocol {
malformed!(":path in CONNECT");
}
// This cannot be empty
if path.is_empty() {
malformed!("malformed headers: missing path");
}
let maybe_path = uri::PathAndQuery::from_maybe_shared(path.clone().into_inner());
parts.path_and_query = Some(maybe_path.or_else(|why| {
malformed!("malformed headers: malformed path ({:?}): {}", path, why,)
})?);
} else if is_connect && has_protocol {
malformed!("malformed headers: missing path in extended CONNECT");
}
b = b.uri(parts);
let mut request = match b.body(()) {
Ok(request) => request,
Err(e) => {
// TODO: Should there be more specialized handling for different
// kinds of errors
proto_err!(stream: "error building request: {}; stream={:?}", e, stream_id);
return Err(Error::library_reset(stream_id, Reason::PROTOCOL_ERROR));
}
};
*request.headers_mut() = fields;
Ok(request)
}

seanmonstar pushed a commit that referenced this issue Jan 20, 2023
This exposes the :protocol pseudo header as Request extension.

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

Successfully merging a pull request may close this issue.

7 participants