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

Allow wss connections #8

Closed
wants to merge 1 commit into from
Closed

Conversation

emieli
Copy link

@emieli emieli commented Jun 10, 2021

ws:// is only allowed if the webpage is served over HTTP. If serving the webpage over HTTPS then a wss:// URL must be sent to the browser.
To do this we attempt to load the ws:// URL first and if the browser throws a DOMexception then we "catch" it and attempts to setup a wss:// URL instead.

ws:// is only allowed if the webpage is served over HTTP. If serving the webpage over HTTPS then a wss:// URL must be sent to the browser.
To do this we attempts to load the ws:// URL first and if the browser throws a DOMexception then we "catch" it and attempts to setup a wss:// URL instead.
Turbo.connectStreamSource(new WebSocket(`ws://${location.host}/turbo-stream`));
} catch(error) {
Turbo.connectStreamSource(new WebSocket(`wss://${location.host}/turbo-stream`));
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to get the correct scheme from the page, I should have done that and forgot.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the request.scheme variable in Flask? The problem with that is that the client may be connecting with HTTPS to an Nginx proxy frontend and Nginx then speaks HTTP to the Flask backend, giving incorrect info about what scheme the browser is actually using.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That problem has a solution, the ProxyFix middleware addresses that. But if you prefer, you can look at the scheme in the browser side.

Your solution is not good because it prefers ws:// over wss://.

@miguelgrinberg miguelgrinberg added the question Further information is requested label Jun 10, 2021
@emieli emieli deleted the patch-1 branch June 28, 2021 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants