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

Make handling of 'root_path' in dispatcher middleware adhere to ASGI spec #259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DaGenix
Copy link

@DaGenix DaGenix commented Sep 29, 2024

Before this change, the dispatcher middleware didn't do anything with 'root_path'. It did, however, modify 'path'.

The problem with this behavior, is that the child ASGI app has no way to determine what its prefix it. And if it can't determine its prefix, it doesn't know how to construct URLs.

The ASGI spec isn't super clear on the expected behavior. However, some resources to review are:

Based on the above, I believe that the correct behavior is that "root_path" should be updated by the dispatcher middleware but that "path" should not be modified.

In addition to the above change, I also updated the tests. And I also added a new test case where the dispatcher middleware is nested inside of itself.

…spec

Before this change, the dispatcher middleware didn't do anything with
'root_path'. It did, however, modify 'path'.

The problem with this behavior, is that the child ASGI app has no way to
determine what its prefix it. And if it can't determine its prefix, it
doesn't know how to construct URLs.

The ASGI spec isn't super clear on the expected behavior. However, some
resources to review are:

* https://asgi.readthedocs.io/en/latest/specs/www.html#wsgi-compatibility
* encode/starlette#2400
* django/asgiref#229

Based on the above, I believe that the correct behavior is that
"root_path" should be updated by the dispatcher middleware but that
"path" should not be modified.

In addition to the above change, I also updated the tests. And I also
added a new test case where the dispatcher middleware is nested inside
of itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant