Opened 11 months ago

Closed 11 months ago

Last modified 9 months ago

#35169 closed Bug (needsinfo)

updated asgi handler for disconnects does not process ROOT_PATH

Reported by: Michael Smith Owned by: nobody
Component: Core (Other) Version: 5.0
Severity: Normal Keywords: asgi, handler, root_path
Cc: Michael Smith, Carlton Gibson, Sören Weber Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

as a result of this PR, https://github.com/django/django/pull/16603, a root path, in this case, set via uvicornm is not handled in the response.

I have created a small PR to fix this issue at https://github.com/msmitherdc/django/pull/1.

Change History (13)

in reply to:  description comment:1 by Michael Smith, 11 months ago

Replying to Michael Smith:

as a result of this PR, https://github.com/django/django/pull/16603, a root path, in this case, set via uvicornm is not handled in the response.

I have created a small PR to fix this issue at https://github.com/msmitherdc/django/pull/1.

django pr is https://github.com/django/django/pull/17828

comment:2 by Michael Smith, 11 months ago

Summary: updates asgi handler for disconnects does not process ROOT_PATHupdated asgi handler for disconnects does not process ROOT_PATH

comment:3 by Mariusz Felisiak, 11 months ago

Cc: Carlton Gibson added
Resolution: needsinfo
Status: newclosed

Thanks for the report, however, I'm puzzled, process_request() has never accepted or passed a scope (even before 1d1ddffc27cd55c011298cd09bfa4de3fa73cf7a), so how this can be a regression? Also, scope is an element of request so why you need to pass it separately and call set_script_prefix(). This should be already handled by ASGIHandler.

comment:4 by Carlton Gibson, 11 months ago

Root path handling was adjusted in Uvicorn just recently. https://github.com/encode/uvicorn/pull/2213

comment:5 by Michael Smith, 11 months ago

all i can say is that when we upgraded to django 5 from 4.2.9, all the template url references outside of static files lost their root_path prefixes. By making these changes I was able to get the root_path passed back to the templates. If you have another way to resolve this, please but I have not been able to get the ROOT_PATH from uvicorn asgi server passed back to our application without this. I used git bisect to identify the specific commit that caused this. And by adding the scope, it restored the django 4 behavior.

comment:6 by Mariusz Felisiak, 11 months ago

all i can say is that when we upgraded to django 5 from 4.2.9

Are you sure that it's not related with the uvicorn update?

I used git bisect to identify the specific commit that caused this.

Can you add a regression test? You had to have one to actually bisect.

comment:7 by Michael Smith, 11 months ago

I will add a regression test

comment:8 by Michael Smith, 11 months ago

It may be that process_request does not need scope but just run_get_response. I will determine what exactly is needed with the regression test

comment:9 by Carlton Gibson, 11 months ago

@Michael. set_script_prefix targets a Local, so if we change thread maybe the value gets lost, but I'm not seeing yet how that can be happening (in anything related to 1d1ddffc27cd55c011298cd09bfa4de3fa73cf7a) — the create_task call uses the current event loop, so the same thread... 🤔 — Happy to have a look once you've got a reproduce. Thanks!

Update: from the asgiref.local.Local doctoring:

In async threads, local means in the same sense as the contextvars

module - i.e. a value set in an async frame will be visible:

  • to other async code await-ed from this frame.
  • to tasks spawned using asyncio utilities (create_task, wait_for, gather and probably others).
  • to code scheduled in a sync thread using sync_to_async

So, exactly, the move to using a subtask (for disconnect handling) should make no difference.

Version 2, edited 11 months ago by Carlton Gibson (previous) (next) (diff)

comment:10 by Sören Weber, 10 months ago

Cc: Sören Weber added

comment:11 by AJ Slater, 9 months ago

I've run into the same problem, except with hypercorn.
script prefixes are no longer prefixed from hypercorn root_path with Django 5.0.0+ They are prefixed with Django 4.2.11 and the same hypercorn version.

My workaround right now is to manually call set_script_prefix() in my urls.py:

from django.urls import set_script_prefix
from myapp.settings import MANUALLY_PARSED_HYPERCORN_SETTINGS

set_script_prefix(MANUALLY_PARSED_HYPERCORN_SETTINGS.root_path) 

urlpatterns = [ ... ]

comment:12 by Carlton Gibson, 9 months ago

I've created a PR adding a test for this here. The test passes (on both main and stable/4.2.x branches).

Comment from there:

Possible change may be in 041b0a3 @sarahboyce: maybe folks have FORCE_SCRIPT_NAME set to a non-None value. That's the sort of thing one might have done when experimenting with mounting under a root_path.
If it's not that, then I'm out of ideas without a reproduce.

Last edited 9 months ago by Carlton Gibson (previous) (diff)

comment:13 by Sarah Boyce <42296566+sarahboyce@…>, 9 months ago

In 4d2ef9b:

Refs #35169 -- Added test for ASGIRequest root_path handling.

Note: See TracTickets for help on using tickets.
Back to Top