django / daphne

Django Channels HTTP/WebSocket server
BSD 3-Clause "New" or "Revised" License
2.4k stars 268 forks source link

Daphne ignores "headers" section from "websocket.accept" message #326

Open albertas opened 4 years ago

albertas commented 4 years ago

I want to add permessage_deflate Websocket extension support for django_channels consumer/server. Server has to provide supported extensions in the headers of handshake response, e.g. Sec-Websocket-Extensions: permessage-deflate. ASGIREF documentation states, that "headers" section is supported for "websocket.accept" message type. Hence, I have tried to accept Websocket connection with this code:

from channels.generic.websocket import AsyncJsonWebsocketConsumer

class CeleryTaskConsumer(AsyncJsonWebsocketConsumer):
    async def connect(self):
       await self.base_send({
           "type": "websocket.accept",
           "headers": [
               (b"sec-websocket-extensions", b"permessage-deflate")
           ]
       })

However, the daphne server ignores the "headers" section of this message and only uses "subprotocol" section: https://github.com/django/daphne/blob/master/daphne/ws_protocol.py#L185

That means, that Daphne server has no way to inform client, that it supports some kind of Websocket extensions, which is a major limitation.

pip freeze:

asgiref==3.2.10
attrs==19.3.0
autobahn==20.7.1
Automat==20.2.0
cffi==1.14.1
channels==2.4.0
constantly==15.1.0
cryptography==3.0
daphne==2.5.0
Django==2.2
hyperlink==19.0.0
idna==2.10
incremental==17.5.0
pyasn1==0.4.8
pyasn1-modules==0.2.8
pycparser==2.20
PyHamcrest==2.0.2
pyOpenSSL==19.1.0
pytz==2020.1
service-identity==18.1.0
six==1.15.0
sqlparse==0.3.1
Twisted==20.3.0
txaio==20.4.1
zope.interface==5.1.0
carltongibson commented 4 years ago

Hi @albertas. Thanks for the report. Looks like we need to update for the 2.1 version of the websockets spec. Fancy adding handling there?

albertas commented 4 years ago

Sure, I will try adding "headers" section support myself. However, I don't know what are other changes in 2.1 websocket spec, hence they will be out of my PR scope.

carltongibson commented 4 years ago

Hey @albertas: no problem! Getting one thing at a time is plenty (but I think headers was the only change IIUC).