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

Accept http/https schemes in URIs for tungstenite::client::{connect, client} #451

Open
alex-kattathra-johnson opened this issue Nov 10, 2024 · 7 comments

Comments

@alex-kattathra-johnson
Copy link

Can we allow connect to http(s) url for endpoints that are upgraded?

@daniel-abramov
Copy link
Member

connect() works for both HTTP and HTTPS URLs (provided TLS features are enabled).

If by "endpoints that are upgraded" you mean running tungstenite over the stream that has already been upgraded to a WebSocket connection, then you can use one of the following functions:

@alex-kattathra-johnson
Copy link
Author

alex-kattathra-johnson commented Nov 11, 2024

We throw an error here for everything that isn't wss or ws. Would be nice if we could allow http and https which could be upgraded into websocket at handshake.
https://github.com/snapview/tungstenite-rs/blob/master/src/client.rs#L148

@daniel-abramov
Copy link
Member

Ah, now I understand what you mean. We expect the URI to have a schema of ws or wss as these are the only ones allowed by the RFC. If your URI contains a different scheme but your endpoint would work fine with WebSockets, you can change the scheme before calling a client() function (or connect() for that matter).

Alternatively, if the upgrade to the WebSockets happens at a later point in time ('custom' protocol or else), you could always use the previously mentioned functions to construct a WebSocket from the stream.

That being said, I understand the inconvenience for certain rare use cases and AFAIK some WebSocket clients accept http/https schemas for the WebSocket URIs, although these are not yet universally supported. In that respect, I'm not against extending our implementation to accept http/https URIs in addition to ws/wss URIs (unless there are strong reasons not to do this).

@daniel-abramov daniel-abramov changed the title tungstenite::client::connect on http(s) url Accept http/https schemes in URIs for tungstenite::client::{connect, client} Nov 11, 2024
@alex-kattathra-johnson
Copy link
Author

The kubernetes watch API currently streams events over http using chunked responses, but upgrades to websocket if the handshake headers are set.
https://github.com/kubernetes/apiserver/blob/cccad306d649184bf2a0e319ba830c53f65c445c/pkg/endpoints/handlers/watch.go#L179

@alex-kattathra-johnson
Copy link
Author

Updating the scheme to ws/wss fixed my specific issue with the kubernetes watch API, FWIW. We can close this issue if we find that allowing non-ws scheme in a websocket client is against the RFC guidelines.

@daniel-abramov
Copy link
Member

Glad it worked for you!

As for the issue - following my previous comment, I'm not opposed to implementing this change. As I mentioned, we did initially stick to ws/wss schemas as these were the ones defined in the RFC, but strictly speaking, http/https are not forbidden and many client libraries and browsers support it, e.g. the JavaScript's WebSocket allows ws, wss, http, https. While it's not universally supported everywhere, in the past years more implementations have adopted this.

@agalakhov
Copy link
Member

Agree, but this has to be feature-gated. There are potential use-cases where it could be unexpected and even security critical, mostly if the address comes from an user input field.

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

No branches or pull requests

3 participants