Daphne-Root-Path not taken in consideration in consumers URLs

[Kerl13]

Daphne does not seem to use the Daphne-Root-Path when it computes the urls of the consumers.

I have:

• Daphne-Root-Path set to appname
• A web page at url r"^home" in my urls.py
• A consumer routed at r"/ws/something" in my routing.py

And I observe that:

• My web page is served at address /appname/home which means that Daphne is using the root-path correctly.
• The websocket is served at /ws/something and not at /appname/ws/something which I would expect.

Indeed, if I look at the source, I can see that http_protocol.WebRequest has a root_path attribute which is used when a http answer is sent (here https://github.com/django/daphne/blob/1.3.0/daphne/http_protocol.py#L162) but there is nothing like this in ws_protocol.py.

Is this actually a bug or am I missing something?

I tested this with Daphne 1.2.0 but it seems that the issue remains on the lastest version on the repository.

[andrewgodwin]

Yes, this is a bug. There's likely also a "bug" in the Channels routing code, in that it does not respect the root_path value but instead patches on the raw path (as it's not URL routing right now, but key matching). That will be harder to fix.

[aureplop]

As fast workaround, we could add path_info to message in Daphne, instead of doing this resolution in Channels. We can later use this to filter messages in Channels routing by using path_info instead of path.
But ASGI message spec doesn't speak of path_info, instead we should have path and root_path set in messages on websocket.connect channel. By the way, why root_path doesn't appear in the spec on websocket.receive and websocket.disconnect channels ?

Also, another consequence for Channels is that urls reverse should not work in websocket consumers, as set_script_prefix from django.urls is never used (which is done by ASGIHandler for message on http.request, as in Django WSGIHandler). Note that I didn't check this for real.

Fix this (for Daphne) should be easy, we only have to add root_path in websockets messages. For Channels, if we want to keep the routing filters as generic as it is now, path filtering should be done with path_info instead of path, which is consistent with Django BaseHandler resolver (which use path_info).

[andrewgodwin]

I wanted to try and avoiding overloading the receive and disconnect channels with information, but I agree, path_info would be better to have than root_path given the circumstances. (Since you only need one, I chose root_path initially as it means you don't need to derive the root from the other two URLs).

The other option is to improve Channels' routing to actually understand URLs properly and deal with path prefixing (this will also let us fix the inconsistency with leading slashes vs. Django URLs that currently exists). It's a bit of a bigger piece of work, but likely a better overall result.

[muhammedabad]

Just to make my issue is related (please correct me if I'm wrong).

On localhost with runserver, I can reach the websocket connect function in my consumer with the following:

# Client
var ws = new WebSocket("ws://localhost:8000/chat/<user_id>/");

# Router config
route("websocket.connect", ws_connect, path=r"^/(?P<user_id>[^/]+)/$"),

On my production environment, my nginx conf for the websocket proxy looks like this:

location /ws/ {
	proxy_pass http://daphne-myapp;
        proxy_http_version 1.1;
        proxy_set_header Upgrade $http_upgrade;
        proxy_set_header Connection "upgrade";
	proxy_set_header Daphne-Root-Path /ws;
	
}

I then try to connect to ws://myapp.com/<user_id>/ but the consumer connect never gets called. After changing the route to include the root path, everything appears to work as expected:

route("websocket.connect", ws_connect, path=r"^/ws/(?P<user_id>[^/]+)/$"),

If my issue is valid and related to this, what would be a suitable workaround ? I could easily create different routes to be used in development and production if needed.

[andrewgodwin]

Yes, that looks like the same issue. For now the workaround is as you showed with the URL config; you could move the prefix part into a setting, maybe, and then use that in the URL config, but really I just need to find some time to fix this.

[ernestkahnwald]

Any updates?

[asedeno]

So, it looks like all we need to do to resolve this is have daphne handle the Daphne-Root-Path header in ws_protocol.py, putting it into the scope as root_path, and falling back on the command line argument --root_path via server.root_path. Since ASGI does not strip it from scope['path'], the rest would need to be handled downstream in channels. Like the http_protocol.py handling, we can also drop the Daphne-Root-Path header from scope["headers"].

I have not dug into channels prior to 4.0 on this particular topic.

In channels.routing.URLRouter, error handling suggests that if there is no scope["path_remaining"] then the URLRouter is the outermost URLRouter.
A similar check could be added earlier, triggering scope["root_path"] stripping and perhaps raising a ValueError if not path.startswith(scope["root_path"]).

I have tested such changes locally with roost-ng, dropping the middleware I wrote to work around this issue in the past, and it seems to work.

@andrewgodwin, is there anything obvious missing here? If not, I'll open PRs with daphne and channels.

[carltongibson]

@asedeno If you open PRS with test
cases it's likely easier to think about than just in a comment description.

I'm targeting new releases for the new year period, so if you break ground I'm happy to consider this for those.

Thanks.

[asedeno]

@carltongibson the implementation as mentioned is in the PRs that are referencing this bug. PTAL when you have a chance.

[asedeno]

@carltongibson: friendly ping.

[carltongibson]

@asedeno "new year period" -- please don't do "friendly pings" it's just noise on my timeline. Thanks.

[asedeno]

@carltongibson I interpreted "targeting new releases for the new year period" as needing to get things in before then, and expect that to take some time, feedback, and iteration. As the sum total of the changes so far are +31-2 for daphne, and +49-0 for channels, mostly in tests, was hoping for feedback sooner rather than later. If you don't actually plan to look at this for weeks, then please set that as the expectation. In the mean time, it would at least be beneficial to unblock workflows for the PRs, so they get tested now and moving forward if I decide to make further changes. Thanks.

[asedeno]

@carltongibson, so the entire new year has passed and we're in a new "new year period" so I figure it's finally time to circle back and see if we can get an update on this.

[carltongibson]

Hi @asedeno — Yes, thanks for your patience. (2023 was something of a year TBH.)

Let me review this and we'll see where we're at.

[carltongibson]

Fixed (on the Daphne side) in #453.

django/channels#1954 Provides the Channels part to go with it.