heroku-python / flask-sockets

[DEPRECATED] Alternative: https://github.com/miguelgrinberg/flask-sock
MIT License
1.74k stars 167 forks source link

Set websocket=True for Rule, fixes #81 #82

Open tomviner opened 3 years ago

tomviner commented 3 years ago

This fixes the WebsocketMismatch which appears with Werkzeug 2.0.0.

The cause is a websocket request hits a rule which Werkzeug doesn't think is a websocket endpoint.

tomviner commented 3 years ago

Unfortunately, Werkzeug pre 2.0.0 doesn't always label websocket requests correctly, so with this PR, you can end up with the opposite mismatch.

Here's the change to Werkzeug where more websocket requests get labelled correctly: https://github.com/pallets/werkzeug/pull/2053/files#diff-9b2e55f893697ae8ac344aebf17bebee83546fc2bf4d8e1b138cf7c63e92080fR1637-R1641

lnstadrum commented 3 years ago

Hi @tomviner,

I ended up with a similar change to make things work on my side, but in addition to your commit I also had to add the following change:

@@ -27,7 +27,7 @@ class SocketMiddleware(object):
     def __call__(self, environ, start_response):
         adapter = self.ws.url_map.bind_to_environ(environ)
         try:
-            handler, values = adapter.match()
+            handler, values = adapter.match(websocket=True)
             environment = environ['wsgi.websocket']
             cookie = None
             if 'HTTP_COOKIE' in environ:

In my tests this makes WebsocketMismatch disappear for a web page socket and a socket opened in Android with Kotlin Ktor API. Not sure if this is universally helpful though... My setup was as follows:

atzannes commented 3 years ago

@tomviner thanks for this fix. It looks like it fixes most of my problems with the Flask upgrade.

Unfortunately, Werkzeug pre 2.0.0 doesn't always label websocket requests correctly, so with this PR, you can end up with the opposite mismatch.

Here's the change to Werkzeug where more websocket requests get labelled correctly: https://github.com/pallets/werkzeug/pull/2053/files#diff-9b2e55f893697ae8ac344aebf17bebee83546fc2bf4d8e1b138cf7c63e92080fR1637-R1641

Maybe also add the constraint Flask>=2.0 in setup.py then.

Rosuav commented 3 years ago

If there's an issue with misrecognizing sockets, maybe the solution would be to use the options kwargs instead?

self.url_map.add(Rule(rule, endpoint=f, **options))

And then you'd use it thus:

@sockets.route("/countdown_ctrl", websocket=True)
def control_socket(ws):

Older versions of flask_sockets would ignore that, so an app that sets this flag would be compatible with both.

hansehe commented 2 years ago

If there's an issue with misrecognizing sockets, maybe the solution would be to use the options kwargs instead?

self.url_map.add(Rule(rule, endpoint=f, **options))

And then you'd use it thus:

@sockets.route("/countdown_ctrl", websocket=True)
def control_socket(ws):

Older versions of flask_sockets would ignore that, so an app that sets this flag would be compatible with both.

This solution would solve it, so someone really needs to fix that! It's an easy fix, and would help a lot. For anyone looking for a temporary solution, then you could do this: ''' sockets.url_map.add(Rule('/mixed_ws_path', endpoint=my_websocket_function, websocket=True)) '''