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

Add support for WebSocket rules in the routing #1709

Merged
merged 2 commits into from Feb 4, 2020
Merged

Conversation

pgjones
Copy link
Member

@pgjones pgjones commented Jan 29, 2020

This allows for Rules to be marked as a WebSocket route and only
matched if the binding is websocket. It also ensures that when a
websocket rule is built with a scheme it defaults to the ws or wss
scheme.

src/werkzeug/routing.py Outdated Show resolved Hide resolved
src/werkzeug/routing.py Outdated Show resolved Hide resolved
src/werkzeug/routing.py Outdated Show resolved Hide resolved
src/werkzeug/routing.py Outdated Show resolved Hide resolved
src/werkzeug/routing.py Outdated Show resolved Hide resolved
src/werkzeug/routing.py Outdated Show resolved Hide resolved
@davidism davidism added this to the 1.0.0 milestone Jan 31, 2020
@davidism
Copy link
Member

@davidism davidism commented Jan 31, 2020

I'm willing to get this in to 1.0.0 since it will enable Quart to depend on Werkzeug, but I'd like to see documentation along the lines of "This is for routing only, Werkzeug itself does not handle websockets. This is useful for ASGI frameworks."

@pgjones
Copy link
Member Author

@pgjones pgjones commented Jan 31, 2020

Great, I've added some documentation with the note/warning.

src/werkzeug/routing.py Outdated Show resolved Hide resolved
src/werkzeug/routing.py Outdated Show resolved Hide resolved
@pgjones pgjones force-pushed the websocket branch 2 times, most recently from 7d0ab59 to 7b5656b Compare Feb 2, 2020
tests/test_routing.py Outdated Show resolved Hide resolved
@davidism
Copy link
Member

@davidism davidism commented Feb 3, 2020

Looking at how new WebSocket behaves in Firefox, it requires a URL with the ws scheme and the host. So build should always build an external URL for websocket rules otherwise they won't work.

Discussing this in chat, we thought about whether WS requests should build external URLs for HTTP rules in the same way, for example in a rejection response. Since browsers assume URLs are relative and HTTP(S) unless told otherwise, we're assuming it's fine not to add this for now.

src/werkzeug/routing.py Outdated Show resolved Hide resolved
This allows for Rules to be marked as a WebSocket route and only
matched if the binding is websocket. It also ensures that when a
websocket rule is built with a scheme it defaults to the `ws` or `wss`
scheme.
@davidism davidism merged commit 13b6ef0 into pallets:master Feb 4, 2020
12 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants