Rocket icon indicating copy to clipboard operation
Rocket copied to clipboard

Websocket channels

Open the10thWiz opened this issue 4 years ago • 32 comments

This is an actual Websocket implementation. Channels, and the ability to easily send messages to a large group of clients has not been fully implemeneted. There are a few issues which need to be resolved before it can be considered complete.

  • [x] Incomming message type
  • [x] Outgoing message type
  • [x] Join & Leave events
    • [x] Does Leave fire when the server initiates the close?
  • [x] Channel descriptor type
    • [x] How to subscribe a client to a channel?
    • [x] How are channels created?
  • [x] Add message size limit
  • [x] Stream messages

Types

Incomming messages are converted to a Data type, and can be converted using any normal FromData type. Outgoing messages must implement IntoMessage, which requires them to identify text vs binary, and convert it into bytes.

IntoMessage currently has a generic implementation for any type that implements AsyncRead. This is likely a bad idea, since it makes implementing IntoMessage for types in the standard library much harder.

Join & Leave

~Should the user be able to specify actions for when a user connects or leaves? This would most likely be implemented via specifying a function to run in the websocket macro.~

These will be seperate macros, specifically a join and leave macro, along with renaming the websocket macro to message. Join and Leave will be optional, although I like the idea of requiring a join event, perhaps with a parameter in the message macro.

Channels

~The current implementation just uses a String, and doesn't implement any type of hierarchy. I think a final implementation needs to have a hierarchy, analogous to Phoenix's topics and subtopics, and I have a few ideas on how this could be acomplished.~

~- String, with optional : as a seperator for topics and subtopics. The only thing this has going for it is that Phoenix does it.~ ~- Path, or a Uri of some type. This has much better support within Rocket already, but may not be ideal.~ ~- A trait, which could perhaps be implemented on user created structs via a derive macro. I like this option the best, however it would require to most work. See Channel Derive below.~ ~- A new struct. I don't like this option becuase it's basically like the Path or Uri option, but without any existing code or support.~

~Subscribing to a topic is done via the subscribe method on a Websocket object, but it could also be implemented as part of the websocket macro. Phoenix requires a client to be subscribed to a topic to be able to send messages to a specific topic, but I don't think it's an absolute neccessity for Rocket, since it would be fairly trivial to check before sending it. To make this easier, we could add a subscribed_to method, which checks whether a client is subscribed to a specific topic.~

I'm looking into creating channels based on the URI used to connect. This would remove the type paramter from Broker (to more closely match Sergio's example), and Broker is due for a complete rewrite anyway.

This will also likely require a custom client-side js library to multiplex connections, since supporting Websockets over HTTP/2 is blocked on h2 supporting the headers we need. This could actually solve the issues with IntoMessage, by enabling us to handle some of the issues on the client-side.

the10thWiz avatar May 13 '21 21:05 the10thWiz

I have finsished my implementation of Phoenix like channels, using a ChannelDescriptor trait, as outlined above. I have not yet written a derive macro for it, but I don't expect it to be very complicated. I have implemented ChannelDescriptor for a set of types in the standard library, so it can already be used without creating a new struct. The implementation allows a simple example where Strings are sufficent to be created using just a String.

There are only a few more things to work on. Join & Leave events, and a potential feature gate are the big ticket item, although a few minor improvements are noted with TODO tags in the code.

the10thWiz avatar May 14 '21 21:05 the10thWiz

For those watching: design discussions are taking place on Matrix. Here is my proposed API for websockets, by example:

//! A multiroom chat application with a special `echo` room which echoes its
//! messages to `lobby`.

// `&Channel`, `&Broker` are channel guards. `&Broker` is also a request guard.
//
// A `Channel` is:
//   1) A bidrectional channel between two endpoints: client and server.
//   2) Linked to a topic URI, for broadcasting to that topic.
//
// A `Broker` knows about all channels.

type UserList<'r> = State<'r, ConcurrentHashSet<String>>;

enum Event<'r> {
    Join(&'r str),
    Message(&'r str),
    Leave(&'r str),
}

/// A new client has connected to a room. Inform the room of this, say hello to
/// the client with a "join" message.
#[join("/<_>", data = "<username>")]
fn join(channel: &Channel, username: &str, users: UserList<'_>) {
    if users.contains_or_add(username) {
        channel.send(Event::Leave("username taken")).await;
        return channel.close();
    }

    channel.local_cache(|| username.to_string());
    channel.send(Event::Join("hello!")).await;
    channel.broadcast(Event::Join(username)).await;
}

/// A message sent to a room that's not `echo`.
#[message("/<_>", data = "<msg>")]
fn room(channel: &Channel, msg: &str) {
    channel.broadcast(Event::Message(msg)).await;
}

/// A message sent to `/echo`: send it to `/echo` and `/lobby` concurrently.
#[message("/echo", data = "<msg>")]
fn echo(broker: &Broker, channel: &Channel, msg: &str) {
    let lobby_send = channel!(broker, room("/lobby")).broadcast(Event::Message(msg));
    let room_send = channel.broadcast(Event::Message(msg));
    join!(lobby_send, room_send).await;
}

/// A user has left a room. Let the room know. `Username` retrieves the username
/// stored in `join` in `channel-local cache` via an elided channel guard impl.
#[leave("/<_>")]
fn leave(channel: &Channel, username: &Username, users: UserList<'_>) {
    users.remove(username);
    channel.broadcast(Event::Leave(username)).await;
}

SergioBenitez avatar May 16 '21 19:05 SergioBenitez

I've done some work on this, and I will be adding it soon. I'm also going to be updating the top message to make it better match the current plan.

the10thWiz avatar May 25 '21 19:05 the10thWiz

I think the Rocket side has a more-or-less completed API. There are only a few changes I want to make, which should be fairly minor. However, the client library has not been written yet. I will need help to make that happen.

  • [x] Seperate Channel Guards from Request Guards
  • [x] Implement cross-channel broadcasts
  • [x] Autobahn testing
  • [x] Leave events
  • [ ] Documentation
  • [x] Panics & internal improvements

Channel Guards should become a seperate type from Request Guards, but we should provide a defualt implementation for T: FromRequest.

The internals for cross-channel broadcasts already exist, but an API needs to be created. Sergio's example treats Channel and Broker as seperate types, but my current code makes Broker a member of Channel.

Unfortunatly, in order to implement the ability to stream messages, I had to write a significant amount of the implementation in Rocket itself. In order to test performance and correctness, we should run the Autobahn test suite against the Rocket implementation.

At the moment, a Leave event is fired even if the server initiated the close. I think this is the desired behaviour, but I want to make sure.

Documentation is sorely lacking right now. I have been putting off writing in depth documentation and examples, especially since the Rocket API was still undergoing major changes. I would like to make an attempt to stablize the API provided before delving into writing documentation.

A finalized version of this implementation should never panic, although I cannot say for sure that it doesn't right now. Testing with Autobahn and writing examples are likely to find some edge cases, but I, and someone else, should manually review the code to find any location where a panic is possible. I would also like to make internal improvements, to either make the implementation more effiecent, or easier to read. There is a large select! statement that is hard to work with. There are some sematics around select! that provide some subtle pitfalls, which should be documented and/or worked around.

@SergioBenitez, Would you mind reviewing the completed portion of this implementation? I would also appreciate any advice or guidance on writing the JS client library for multiplexing connections. I have documented the protocol at the end of channels::router, but it needs a permanent home. I suspect it should be added to the guide, but I haven't gotten to adding websockets to the guide yet.

the10thWiz avatar Jun 02 '21 00:06 the10thWiz

I think this PR is feature complete. It has all of the features I want to include, let me know if there are any omissions that need to be addressed.

There are a number of changes that need to take place before this is ready, however they should be internal or minor changes, such as explicitly handling more situations, or changing the api. I'm not ruling out breaking changes yet, but I would like to validate that all of the required features are present.

Specific notes

Panics

I need to explicitly handle panics in user handlers

Autobahn testing

I have run the Autobahn test suite agaisnt the websocket implementation in Rocket, and there are a number of tests that fail. See the channels module documetation for more information.

Rocket-Multiplex

I have written a full specification of the rocket-multiplex protocol, in the rocket_multiplex module. I have also implemented it in Rocket, and a client JS library (It's in the websocket-multiplex example right now).

It does currently prevent the use of any other subprotocol, but I also haven't implemented it on the server side, so I don't think it will be an issue.

Guide

I have written a single page for the guide, but it's not complete in any way. It was nice just to make notes and put it in writing, and it helped me find pain points in the API.

IntoMessage

I have chosen to switch to individual IntoMessage implementations, rather than a blanket implementation on T: AsyncRead. This makes it possible to implement it on Json Values, as well as String and other types in the standard library.

Websocket Macros

There are changes need to the websocket macros. I will likely need to change some of the call signatures and codegen, as well as the macros parameters.

the10thWiz avatar Jun 08 '21 18:06 the10thWiz

Is there going to be a rust client built on web-sys, native, or others as well? It would be very useful to be able to access it directly from within wasm apps in the browser for consistency and speed or via native clients as javascript doesn't really exist there.

OvermindDL1 avatar Jun 15 '21 15:06 OvermindDL1

@OvermindDL1, are you looking for something like wasm-sockets? I think it would be a nice test to check if this PR and wasm-sockets are compatible.

schrieveslaach avatar Jun 16 '21 07:06 schrieveslaach

@OvermindDL1 @schrieveslaach Yes, this PR will be compatible with wasm-sockets, and a variety of other client libraries. They are both based on RFC6455, and will work together.

I am also using Autobahn|Testsuite to test this PR's compliance with the RFC, so there should be no issues.

the10thWiz avatar Jun 16 '21 16:06 the10thWiz

@SergioBenitez I have made some major changes. I just posted a message on Matrix to this effect, and I'm going to talk about them in some more detail here.

  • Removed the WebSocketRouter, prefering to integrate it into the Router.
  • Removed the need to share WebSocket Requests in Arc, by writing better lifetime bounds.
  • Changed the Join event to be run on the first message.

Join events

If the Join event forwards (either because there was not matching handler, or the guards forwarded), the message is then run as a Message event. This means that if a Join event matches, it gets the first message sent on the channel, while keeping the Join event optional.

Client side code would then look something like this:

let ws = new WebSocket(path);
ws.onopen = () => { ws.send(token); };

The send method can't be called until the connection is opened, so this code can't be simplified, but a Rocket specific client library could provide a convience method to open a connection and enqueue the first message.

Previously, I had expressed reservations about this pattern, since I was under the impression that the error event would not be run if the connection was closed by the server after the handshake. After doing some testing, it looks like I was completely wrong; the error event does not run UNLESS the connection completes the handshake. Therefore, failing the connection after the handshake is actually preferable.

This does create an interesting issue for broadcasts: the client CANNOT be subscribed to a topic until it has sent a message. Allowing a client to be subscribed earlier opens up the possiblity of the client recieving messages from topics it isn't authorized to, since the guards aren't run until the first message. However, this should be fairly trivial to solve in practice, by just sending some sort of hello world message immediatly after joining.

Regressions & Todo

These changes also introduce some regressions to the implementation.

  • [ ] Topic multiplexing
  • [ ] Broker efficiency
  • [ ] Documentation
  • [x] Autobahn CI

I haven't implemented topic multiplexing yet, although doing so should be fairly trivial once I port some of the parsing code over from the old implementation.

The Broker implementation is not particularly efficient, but this is an implementation detail that can be changed later.

I've abandonded most of the documentation I wrote for the previous implementation, partially because most of it is now outdated.

I have run Autobahn locally against my implementation, and there are no new failures. The implementation still fails 5.19 or 5.20, due to a race condition between concurrent tasks.

the10thWiz avatar Jun 18 '21 15:06 the10thWiz

@SergioBenitez I have been thinking about the Broker and Multiplexing. I believe the central reason to attempt to create a Multiplexing protocol was because we needed a way for a client to join multiple topics without creating multiple connections. This would be trivial with HTTP/2, since we could just rely on HTTP/2 to multiplex for us. However, since h2 and many browsers don't support WebSockets over HTTP/2, the next alternitive was creating a Multiplexing protocol. However, this creates a number of the issues we have discussed on Matrix; It forces us to create some shared state between the channels being multiplexed, and requires some odd things with the Request object.

I think we may be able to avoid multiplexing altogether by providing the ability to select a subset of client connected to a Uri. Even if we did support a multiplexing protocol, we are forced (by the WebSocket protocol) to handle messages on each connection in serial, so multiple connections may still be desireable. For this reason, I don't think a multiplexing protocol would result in a single WebSocket connection from each client, rather it would reduce the number connections. Therefore, I think it may be eaiser to handle with some type of subtopic system, so a broadcast can be sent to a subset of clients connected to a topic. This would allow a more complex application, such as a chat room, to write more complex bounds on which clients recieve the broadcast, while a simpler application, such as a notification system, would be able to write much simpler bounds.

I've been working on how I would want to implement this, and I have some ideas. Internally, I think it would most likely come down to a fn(&Request<'_>) -> bool, which would be used to select clients. Externally, it would be generated with a macro, which I will call channel.

// Simple URI
channel!(broker, "/room/lobby")

// handler based -> more or less the same as the URI, but an underscore allows a wildcard for
// that slot
channel!(broker, echo(...))

// Total control
channel!(broker, "/room/{age}", |age: u8| age < 5)

Syntax: BROKER, MATCHER => where BROKER is a variable refering to the broker

MATCHER: (STRING-LIT, ARGS?) | HANDLER_FN => where STRING-LIT is a literal URIs

HANDLER_FN: HANDLER '(' ARGS,* ')' => where HANDLER is the name of a route, and ARGS is a set of arguements, similar to the uri! macro

ARGS: ( '|' PARAM,* '|' CODE? ) | PARAM,* => where CODE is a valid Rust expression that evalueates to a bool or bool-like object (i.e. Result, Option, or Outcome)

PARAM: NAME ':' TYPE => where NAME is an identifier and TYPE is a Rust type

Some notes about the syntax: I wanted to avoid just inventing syntax, so every part is (arguably) valid Rust code. The first two args are the broker and the URI, in one of several forms. The last argument can be something that looks like a lambda expression, or just the list parameters. This does have the downside that you can't call an external function, but I think it should be reasonable to create a secondary macro, something like channel_selector, which is the same, but doesn't accept a broker. The user would then need to combine it with a broker (probably via .with(&broker)), but it would be possible to assign it to a const variable, or something else.

This would be expanded to something like:

{
   function inner<'r>(request: &'r Request<'_>) -> bool {
      #path_guards
      #param_guards
      #request_guards
      #user_code|true
   }
   InnerChannel(&broker, inner)
}

This creates some convient options: some type of ChannelLocal values would be handy, since they could be used to identify and assign complex information about a client. However, I'm not sure how to implement some like that yet, and I'm not sure I want to try.

the10thWiz avatar Jun 23 '21 00:06 the10thWiz

@OvermindDL1, are you looking for something like wasm-sockets? I think it would be a nice test to check if this PR and wasm-sockets are compatible.

In short yes. Being able to talk to rocket 'channels' via a rust API (running in WASM) instead of a JS channel API (compared to having to reimplement the rocket 'channel' api on top of standard websocket) would be a huge boon in unifying code. Nice to hear!

OvermindDL1 avatar Jun 28 '21 17:06 OvermindDL1

Will there also be a way to open standard websockets without a channel-like API, sometimes we have to use client libraries that we don't control how they talk over websocket so we have to have a base websocket interface.

OvermindDL1 avatar Jun 28 '21 17:06 OvermindDL1

Will there also be a way to open standard websockets without a channel-like API, sometimes we have to use client libraries that we don't control how they talk over websocket so we have to have a base websocket interface.

Yes? As it stands, this will implement RFC 6455, and potentially a protocol on top of RFC 6455 that to provide topic multiplexing. Topics are identified by the Origin URI used to connect, and there is (by default) no special marking done. The potential multiplexing protocol is a layer on top of the RFC, and is just some information pre-pended to the actual message. There are discussion happening on Matrix as to the nature and necessity of the Multiplexing protocol, and how it will be handled serverside, but we do not have a consensus yet.

For those following along here on Github:

Basically, multiplexing is hard, because of the Request object, and joining channels. We have a few options, but none of them are particulaly good. We can virtualize the Request object, which means it will be shared across every connection in the channel. This means that the Request's local_cache will be shared, and cannot store seperate values for each channel. We could choose to duplicate the Request object, and store one for every topic the client has joined. This would make the Request's local_cache specific to each topic, but this may not be desirable.

In the end, I think it is best to virtualize the Request, and provide a seperate local cache specific to the topic. This should be clearly called out in the docs, but I can also virtualize the Request without providing a shared cache between topics.

This also complicats Joining. Joining need to be finalized, since there is likely room for causing issues. At the moment, Rocket treats the first message on a channel as the Join message, and refuses to subscribe the client to the topic until they send their first message. I suspect that this is somewhat unituitive, but it should be reasonably simple to solve, typically the client just sends a hello message.

the10thWiz avatar Jul 02 '21 16:07 the10thWiz

For those following along, I have been doing some research into how WebSocket authentication can be done. I have found an option that I like, and think Rocket should support: temporary urls. Basically, the client makes an HTTP request to get a temporary url, and starts the WebSocket connection with that url. The actual authentication is handled by the HTTP route, which is already extensively supported by Rocket. This also helps enormously on the client side, since the vast majority of WebSocket client libraries don't allow the user much freedom to control the handshake process.

There are obviously some differences to handling authentication this way. Previously, I had discussed (and currently implemented) using the first message on a channel as the join message. This provided data for the join handler, but this still suffered from some other issues. This authentication setup would replace the join handler's data, which means join can be called immediatly after the connection is established, without waiting for the client to send a message. To facilitate this as a complete replacement, the token will be able to carry data from the authentication route to the event handlers.

Here is a complete example that provides a basic authentication setup, and how the code will actually look:

#[macro_use] extern crate rocket;

use rocket::http::Status;
use rocket::websocket::Channel;
use rocket::response::content::Html;
use rocket::websocket::token::WebSocketToken;
use rocket::{State, Data};
use dashmap::DashSet;
use rocket::serde::{Deserialize, Serialize};
use rocket::serde::json::Json;

#[get("/")]
fn index() -> Html<&'static str> {
    Html(r#"<!DOCTYPE html>
<html lang="en">
   <head>
        <title>WebSocket Channel Server</title>
    </head>
    <body>
        <h1>Channel Server</h1>
        <p>Messages sent from here will be sent to every connected client</p>
        <p id="status">Connecting...</p>
        <input type="text" id="text" />
        <button type="button" id="send">Send</button>
        <div id="lines"></div>
        <script type="text/javascript">
            const lines = document.getElementById('lines');
            const text = document.getElementById('text');
            const status = document.getElementById('status');
            // Example get_connection code. It takes a username
            var get_connection = function(name, conn, error) {
                // This creates a web request, and adds the appropriate event listeners
                let req = new XMLHttpRequest();
                req.addEventListener("load", (e) => {
                    if (e.target.status != 200) {
                        // If it failed
                        error(e);
                    } else {
                        // Create the actual connection, and pass it to conn
                        let connection = new WebSocket('ws://' + document.location.host + e.target.responseText);
                        conn(connection);
                    }
                });
                req.addEventListener('error', error);
                // This is the POST request to the auth route below
                req.open("POST", document.location + "/listen");
                // This adds the body. A simpler API might not require JSON, but we do.
                req.send(JSON.stringify({ 'name': name }));
            };
            // This is primarily to prompt the user for a username
            var get = function() {
                get_connection(prompt('Username'), (ws) => {
                    // this callback is called on success
                    ws.addEventListener('open', (e) => {
                        status.innerText = 'Connected :';
                    });
                    ws.addEventListener('close', (e) => {
                        status.innerText = 'Disconnected :(';
                        lines.innerHTML = '';
                    });
                    ws.addEventListener('message', (msg) => {
                        const line = document.createElement('p');
                        line.innerText = msg.data;
                        lines.prepend(line);
                    });
                    send.addEventListener('click', (e) => {
                        ws.send(text.value);
                        text.value = '';
                    });
                    // this callback is called on failure; we just retry
                }, (e) => get());
            };
            get();
        </script>
    </body>
</html>"#)
}

/// User struct (this version just contains the name, but a more complete app might also add
/// profile picture, user id, etc)
///
/// This derives Eq and Hash so it can be used in the DashSet (a concurrent HashSet)
#[derive(Debug, PartialEq, Eq, Hash, Default, Serialize, Deserialize, Clone)]
#[serde(crate = "rocket::serde")]
struct User {
    name: String,
}

// This is the authenication route. By default, WebSocketToken will use the Origin URI of this
// route as the URI of the resulting WebSocket connection, regardless of the actualy URI the client
// used.
#[post("/listen", data = "<data>")]
fn auth(data: Json<User>, users: &State<DashSet<User>>) -> Result<WebSocketToken<User>, Status> {
    // users.insert will return false if the Username is already taken
    if users.insert(data.clone()) {
        // Return WebSocketToken with the requested username
        Ok(WebSocketToken::new(data.into_inner()))
    } else {
        Err(Status::Conflict)
    }
}

// There is no permitted Data for join - any data that need to be passed (like the username here)
// must be passed via WebSocketTokens
//
// This event should only be used for sending a join message to other people connected; no actual
// authentication logic should be here.
//
// TODO: This is currently required (due to a todo!() in rocket), some additional work needs to be
// done to the data attributes.
#[join("/listen")]
async fn join(ws: Channel<'_>, user: &WebSocketToken<User>) {
    ws.broadcast(format!("{} Joined", user.get_data().name)).await;
}

// This is more or less the same as before
#[message("/listen", data = "<data>")]
async fn listen(data: Data<'_>, ws: Channel<'_>, _user: &WebSocketToken<User>) {
    ws.broadcast(data).await;
}

// Same here
#[leave("/listen")]
async fn leave(ws: Channel<'_>, user: &WebSocketToken<User>, users: &State<DashSet<User>>) {
    ws.broadcast(format!("{} Left", user.get_data().name)).await;
    users.remove(user.get_data());
}

#[launch]
fn rocket() -> _ {
    rocket::build().manage(DashSet::<User>::new()).mount("/", routes![auth, join, listen, leave, index])
}

This is a complete (working) example, based on the websocket-auth branch of my fork. The authentication is the simplest possible, just verifying that their username is not already taken, but it would not be extrodinarily hard to add a more complete authentication scheme, especially since the authentication route is a normal HTTP route.

Under the hood

Rocket's side, the implementation is fairly straight forward: The WebSocketToken generates a random token, and pushes it into a HashMap along with the data and uri requested. When the user connects to a special route (/websocket/token/{}), Rocket references the table of tokens and attempts to find one. If found, the server removes the token (so it can only be used once), and updates the WebSocket request to look like the original auth request.

To prevent abuse of this system, token expire after a set period of time (30 secs, this should probably be converted into a configuration value), and expired tokens are collected and disposed of periodically. This period could potentially be configured, but it may be best to simple make it some multiple of the token expiration. This means that normally the map should never get too large, since it should only hold a limited number of connections. However, based on the code above, it could be possible for a clients to make usernames inaccesable, by reserving them, but never connecting. To handle this, Rocket will actually execute the leave handler when an expired token is disposed of, specifically to prevent issues like this.

In theory, this could be implemented with something similar to private cookies, where the token is actually just encrypted information about the connection, but that has a couple of issues: it requires the private cookies feature to be enabled (mostly for the deps), and it also makes it impossible to run the leave handler on expired connections, since they are not saved on the server side. The only dependency this branch adds is dashmap, which is the actual HashMap implementation used. Swapping out the map isn't really an issue, but dashmap appeared to be the best option available.

Improvements

I'm not sure exactly what this will look like in it's final state, but I think there are a few minor improvements I can point out right now:

  • WebSocketToken::with_uri: Adding some support for specifying the URI the WebSocket connection should be routed with. This would not be exposed to the client, but rather just held on the server side. At the moment, the Uri of the authentication request is used.

Multiplexing

The current PR doesn't implement multiplexing, but this could make multiplexing easier. With some under the hood changes, the Request from the authentication route can be re-used as the inner Request of the WebSocket object. A re-written version of the multiplexing protocol could take advantage of this, and rather than opening a new WebSocket connection for every topic, could instead submit the connection token on an existing connection. This neatly sidesteps the need to virtualize requests for multiplexing, since the client is making a separate HTTP request for each connection. This does mean that multiplexing is only possible with an authenticated route, although writing an authenticated route is reasonably simple (4 lines of code).

Edits

  • I've udpated the code so now the WebSocketToken request guard looks like &WebSocketToken<T> rather than WebSocketToken<&T>. This change was fairly simple after I switch to moving the entire local cache rather than just the token itself.

the10thWiz avatar Jul 08 '21 18:07 the10thWiz

For those following along, I have been doing some research into how WebSocket authentication can be done. I have found an option that I like, and think Rocket should support: temporary urls.

What advantages does this have over using a query string parameter, e.g. ws://example.com/my-websocket-endpoint?access_token=auth.token.here (this is how SignalR generally handles authentication over websockets and server-sent-events)

GREsau avatar Jul 09 '21 15:07 GREsau

@GREsau

What advantages does this have over using a query string parameter, e.g. ws://example.com/my-websocket-endpoint?access_token=auth.token.here (this is how SignalR generally handles authentication over websockets and server-sent-events)

The advantage is minimal; from a certain prespective they are equavalent. In Rocket, the URIs would be /websocket/token/<token> vs /my/endpoint?token=<token>.

The only reason I prefer temporary URIs over query parameters is because Rocket uses the URI of the request as a topic identifier. At the moment, I don't believe it's possible to broadcast to everyone connected to /my/endpoint?token=<token>, since you would need to broadcast to each indiviual token. With temporary URIs, Rocket replaces the URI the client connected to with the authentication URI (or one specified by the user). This means that in my example, when you connect to /websocket/token/<token> with a token from /listen, Rocket will replace the URI with /listen before routing WebSocket events.

There are a few downsides to this type of implementation:

  • Rocket needs to reserve a WebSocket path for tokens, however this could be mitigated by choosing the path on startup.
  • The client side code is more complex, however this could be mitigated with examples in a number of languages, or custom client libraries for some. Especially if we do choose to go down the Multiplexing path, writing client libraries is a must.

Overall, the difference is relatively small. There are compromises to going either way, but I think that temporary URIs is slightly better.

Also, this is how Slack's socket protocol works.

the10thWiz avatar Jul 09 '21 16:07 the10thWiz

I've made some major changes.

  • WebSocket authentication code is now part of the PR
    • Join events no longer have Data
  • WebSocket vs Channel has been resolved there is now just one type (Channel)
  • Leave events can now use the close status as Data
  • IntoMessage is no longer needed, Responder is now a supported alternative

Overall, I think this is in a much better place. Some review would be nice, to see if I'm on the right track. To make this easier, I'm going to spend some time writing better examples and information for the guide.

Concerns:

I have a few concerns with the code I've written. I've found a way to avoid the need for IntoMessage, specifially by supporting Responder instead. Basically, I just extract the ContentType to determine whether the type is binary or text, and then forward the body to the client. This does just discard headers and status, and I'm not sure the implications of this.

Major TODOs:

  • [ ] Documentation
  • [ ] Examples
  • [ ] Broker effiency
  • [ ] Investigate Multiplexing

The changes to Broker would be entirely internal, and would not effect the external API. Multiplexing is largely off the table at this point, and I have no specific plans for it. For now, I'm treating Multiplexing as an enhancement rather than a core feature.

@SergioBenitez I think this is ready for a review.

I would like to draw your attention specifically to the Channel type, and the send method. I've noted it above in my concerns.

I hope the API here can be stablized, and the only concern is performance, documentation and enhancements. I think the next major step is writing actual code using this, and trying to find any issues.

the10thWiz avatar Jul 13 '21 18:07 the10thWiz

Is it possible to get raw access to a websocket with this PR (and do your own auth and protocol/channel management)? The channels API sounds great, but I imagine a lot of people wanting to use websockets with Rocket will have an existing protocol implementation that they need to conform to, and won't necessarily have the luxury of converting to Rocket's system.

nicoburns avatar Jul 22 '21 19:07 nicoburns

Rocket won't impose any kind of protocol by default. A "channel" is just the name for an internal handle that represents a topic and a client.

SergioBenitez avatar Jul 22 '21 21:07 SergioBenitez

I think @nicoburns is more referencing when one is forced to use a JS library that does their own client websocket handling, which is more annoyingly common than one would hope...

OvermindDL1 avatar Jul 26 '21 22:07 OvermindDL1

I think @nicoburns is more referencing when one is forced to use a JS library that does their own client websocket handling, which is more annoyingly common than one would hope...

Yes, but there are also situations when you may be forced to make it work with an existing client (i.e. migrating a server to Rocket from another framework).

As far as how much this will work: There are limitations. Implementing your own auth isn't hard, although I may want to bring back the above idea of a matcher to make that work better. I don't expect protocol and channel management to be exposed to the user code.

There are a few considerations: Rocket's system is largely server-side, and I'm specifically designing the code to work with the browser based JS WebSocket library. It's a pretty restrictive client, especially when it comes to authentication, which is why the authentication system is designed the way it is. Out of curiosity, @nicoburns what type of authentication are you using? The two most common types of authentication I've seen are both token based, and the only difference is how the tokens are passed to the websocket connection. One option is a query parameter, but Rocket considers that to be part of the topic, and the other option is a temporary URI (i.e. the entire URI is the token). An alternative to Rocket's WebSocket Token would be to parse the token from a query param, stick it into the request's local cache, and remove it from the URI in a fairing. Then, the route would have a request guard that checks for the existence of the token in the cache.

Custom channel management is somewhat difficult to talk about, because I'm not quite sure what you mean by channel. Rocket thinks of a channel as a set of connections, identified by a topic (the origin URI used to connect). It may be possible to add a few options that would allow for the creation of a separate broker, but I don't think it's worthwhile. If there is some flexibility you are looking for from the Broker, let me know. I'm not very happy with the implementation, and I feel that there must be a better way.

the10thWiz avatar Jul 27 '21 20:07 the10thWiz

Is there any update?

HarmoGlace avatar Sep 12 '21 18:09 HarmoGlace

I can recommend taking a look at axum to those that require built-in websocket support (needs to be enabled via a feature flag). It has similar usage ergonomics like Rocket and is developed and maintained by the tokio team. Further key advantages are that axum is modular, it takes advantage of easy plug&play with tower middleware, it's not a one-man show and less opinionated than Rocket.

toxeus avatar Sep 22 '21 12:09 toxeus

@toxeus That doesn't really seem appropriate to post here? Especially as the reason a lot of people like using rocket is due to how its path resolution and guards work, which last I checked axum has neither functionality of?

OvermindDL1 avatar Oct 01 '21 14:10 OvermindDL1

@OvermindDL1 I don't think there's anything inappropriate about my post.

I'm a rocket user myself but I cannot use it in services where I need built-in websocket support. Axum is a good choice for such use-cases. I think it's just fair to share about options with the community while they are waiting for this PR to land.

toxeus avatar Oct 01 '21 16:10 toxeus

@toxeus The community tends to pretty well know about about axum at this point though, I keep hearing about it all over the place. ^.^;

What would be nice is showing how to use axum and rocket together, to use axum as the websocket interface but rocket for everything else. I 'think' that's possible with hyper as the backend.

OvermindDL1 avatar Oct 01 '21 18:10 OvermindDL1

@OvermindDL1 interesting! I've heard about axum just recently. That's why I felt it's worth sharing about it after I experienced how well designed it is.

What would be nice is showing how to use axum and rocket together, to use axum as the websocket interface but rocket for everything else. I 'think' that's possible with hyper as the backend.

Nope, rocket doesn't allow for that. As I said before, rocket is an opinionated project :)

toxeus avatar Oct 01 '21 21:10 toxeus

Another use-case I just hit with rocket, needing to allow graphql subscriptions at a certain route, and well no websockets, of which the graphql layer needs to take over as it has its own API language to speak (async-graphql) over it to pre-existing web and non-web clients where I cannot change how they work. I reduced that project to axum for now but I really hope that rocket gets some way to work with websockets directly so I can uncomment all that original code...

Nope, rocket doesn't allow for that. As I said before, rocket is an opinionated project :)

I was thinking the other way, axum underneath that delegates to rocket on anything it doesn't handle (well, hyper would do that specifically, not axum). I'm pretty sure rocket has a way for that but I've not tested yet, wanted to get the above thing done pretty asap so I didn't feel like experimenting yet.

OvermindDL1 avatar Oct 19 '21 21:10 OvermindDL1

@OvermindDL1 that is exactly my use-case and I had also to reach for axum to make it work :)

With that said, I feel it's slightly off-topic to discuss ideas on how to make rocket work with axum/hyper in this PR. I assume that people who have subscribed to this PR are mostly interested in the progress of the PR. But please ping me in case you find a solution ;)

toxeus avatar Oct 20 '21 08:10 toxeus

Is there any status updates on this PR? Lack of WS support really seems like the single biggest sticking point for consumers of this framework.

What still needs to be done? @SergioBenitez @the10thWiz

EgMan avatar Feb 14 '22 04:02 EgMan