seanmonstar / warp

A super-easy, composable, web server framework for warp speeds.
https://seanmonstar.com/post/176530511587/warp
MIT License
9.51k stars 713 forks source link

Expose `Sec-WebSocket-Protocol` header value when present #760

Open pfernie opened 3 years ago

pfernie commented 3 years ago

This issue is related to #496, although the use case is slightly more general.

Currently, as documented in that issue, warp handles the WebSocket upgrade statically, and elides any handling of a Sec-WebSocket-Protocol header. A workaround used in #496 and similarly in async-graphql's warp integration is to manually set this header in the response to the assumed protocol (e.g. graphql-ws).

With the recent revision of the GraphQL Over WebSockets "spec", the situation is further complicated. The name of the protocol was changed; from the old de facto standard established by subscriptions-transport-ws the subprotocol was named graphql-ws. With the revised graphql-ws, this is (confusingly) renamed graphql-transport-ws. But beyond wanting to reply with the same protocol as the client set in the Sec-WebSocket-Protocol header of the initial GET request, the new graphql-ws protocol differs slightly from the previous subscriptions-transport-ws, and as such servers wanting to support both legacy and newer clients need to act (change logic) accordingly.

Some solutions which would facilitate this are:

EDIT: Per the RFC, the header may specify multiple supported protocols, so protocol would more appropriately be Option<Vec<String>>, or simply Vec<String> if there is no semantic use to distinguishing empty header from non-present header.

seanmonstar commented 3 years ago

It's probably worth pointing out that this can be done already, it just isn't part of the Ws type.

warp::ws()
    .and(warp::headers::value("sec-websocket-protocol"))
    .and_then(|ws, proto| {
        let reply = ws.on_upgrade(something_with_websocket);
        Ok(warp::reply::with_header(reply, "sec-websocket-protocol", proto))
    })
pfernie commented 3 years ago

Thanks, that is a straightforward solution indeed!

benitogf commented 3 years ago

the headers filters moved, updated solution:

warp::ws()
    .and(warp::filters::header::value("sec-websocket-protocol"))
    .and_then(|ws, proto| {
        let reply = ws.on_upgrade(something_with_websocket);
        Ok(warp::reply::with_header(reply, "sec-websocket-protocol", proto))
    })
thewh1teagle commented 11 months ago

Is there a way to allow both connections which sent with sec-websocket-protocl but also connections which made without this header?

Because I think that warp::filters::header::value("sec-websocket-protocol") allow only connections which has this header

thewh1teagle commented 11 months ago
 let app = warp::path("ws")
        // The `ws()` filter will prepare Websocket handshake...
        .and(warp::ws())
        .and(warp::header::optional::<String>("sec-websocket-protocol"))
        .and(users)
        .map(|ws: warp::ws::Ws, maybe_proto: Option<String>, users| {
            // This will call our function if the handshake succeeds.
            let reply = ws.on_upgrade(move |socket| user_connected(socket, users));
            if let Some(proto) = maybe_proto {
                return warp::reply::with_header(reply, "sec-websocket-protocol", proto)
            }
            return warp::reply::with_header(reply, "", "");
        });

Maybe something like that, not sure. it force me to reply with header eventually