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

Handling of scope["root_path"] and scope["path"] differs from ASGI spec #1336

Closed
2 tasks done
silane opened this issue Nov 15, 2021 · 2 comments · Fixed by #2352
Closed
2 tasks done

Handling of scope["root_path"] and scope["path"] differs from ASGI spec #1336

silane opened this issue Nov 15, 2021 · 2 comments · Fixed by #2352
Assignees
Labels
bug Something isn't working help wanted Feel free to help refactor Refactor code
Milestone

Comments

@silane
Copy link

silane commented Nov 15, 2021

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

Accoring to django/asgiref#229 (comment), scope["path"] includes the scope["root_path"].
So if root_path is /foo and request for /foo/bar has come, the upstream application servers (such as uvicorn) are passing scope["path"] == "/foo/bar" and scope["root_path"] == "/foo", which is correct behavior.

But starlette does not conform this. It assumes scope["path"] is a remainder of stripping scope["root_path"] from original request path. In this case, scope["path"] == "/bar" and scope["root_path"] == "/foo". This ASGI incompatible assumption can be seen here or here and many other places.

The following "To reproduce" section shows just one aspect of this wrong assumption.

To reproduce

  1. Save main.py as follows.
from starlette.applications import Starlette
from starlette.responses import PlainTextResponse
from starlette.routing import Route, Mount


class Bar:
    async def __call__(self, scope, receive, send):
        await PlainTextResponse(f"bar {scope['root_path']=} {scope['path']=}")(scope, receive, send)

class FooBar:
    async def __call__(self, scope, receive, send):
        await PlainTextResponse(f"foobar {scope['root_path']=} {scope['path']=}")(scope, receive, send)

routes =[
    Route('/bar', endpoint=Bar()),
    Mount('/foo', routes=[
        Route('/bar', endpoint=FooBar())
    ])
]
app = Starlette(routes=routes)
  1. Run uvicorn main:app --port 8000 --root-path /foo
  2. Access by curl http://localhost:8000/foo/bar

Expected behavior

Receives

bar scope['root_path']='/foo' scope['path']='/foo/bar'

Actual behavior

Receives

foobar scope['root_path']='/foo/foo' scope['path']='/bar'

Some points here are,

  1. scope['path'] does not include scope['root_path'] , which is ASGI incompatible.
  2. FooBar ASGI handler is called. It does not take scope['root_path'] into account when routing.

Environment

  • OS: Linux
  • Python version: 3.8.2
  • Starlette version: 0.17.0

Additional context

This may be a root cause of this issue in fastapi

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@Kludex Kludex added bug Something isn't working refactor Refactor code and removed bug Something isn't working labels Nov 27, 2021
@Kludex Kludex self-assigned this May 22, 2022
@Kludex Kludex added the bug Something isn't working label May 22, 2022
@Kludex Kludex added this to the Version 0.21.0 milestone Jul 5, 2022
@frankie567
Copy link
Member

Hello @Kludex 👋 I would be interested to help tackling this issue; if you need a hand, let me know :)

@Kludex
Copy link
Sponsor Member

Kludex commented Jul 22, 2022

Take in consideration that I'll need a lot of references to follow on this, and to understand the implications for our users.

But yeah, help is appreciated. 🙏

@Kludex Kludex added the help wanted Feel free to help label Jul 22, 2022
frankie567 added a commit to frankie567/starlette that referenced this issue Jul 22, 2022
Kludex pushed a commit to frankie567/starlette that referenced this issue Sep 11, 2022
@Kludex Kludex modified the milestones: Version 0.21.0, Version 1.x May 31, 2023
@Kludex Kludex modified the milestones: Version 1.x, Version 1.0 Nov 27, 2023
maartenbreddels added a commit to widgetti/solara that referenced this issue Dec 5, 2023
path now includes root_path so we need a different way to
remove it. It seems like route_root_path gives this information.

See encode/starlette#2361
encode/starlette#2352
encode/starlette#1336
maartenbreddels added a commit to widgetti/solara that referenced this issue Dec 8, 2023
path now includes root_path so we need a different way to
remove it. It seems like route_root_path gives this information.

See encode/starlette#2361
encode/starlette#2352
encode/starlette#1336
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Feel free to help refactor Refactor code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants