0.35.0 refactor of root_path handling is potentially returning incorrect route

[gabriel-f-santos]

Discussed in #2495

^{Originally posted by robbielaw February 8, 2024}

When running starlette/fastapi behing a proxy we got some errors when calling the app directly.

when calling curl http://127.0.0.1:8000/products/, we get:
root_path: /product
scope['path']: /products
route_path: s (after apply the regex - https://github.com/encode/starlette/blob/master/starlette/_utils.py#L96)
because the the root_path to be removed if calling directly the app is "product" and endpoint path is /products

Example Scenario:
libs:

starlette==0.37.2
uvicorn==0.29.0

gist: https://gist.github.com/gabriel-f-santos/df75213fe6860e369a1e096611a767e5
run: uvicorn main:app --root-path /product

Code causing the error:
https://github.com/encode/starlette/blob/master/starlette/_utils.py#L96

def get_route_path(scope: Scope) -> str:
    root_path = scope.get("root_path", "")
    route_path = re.sub(r"^" + root_path, "", scope["path"])
    return route_path

Fix suggestion:

def get_route_path(scope: Scope) -> str:
    root_path = scope.get("root_path", "")
    route_path = re.sub(r"^" + root_path + r"(?=/|$)", "", scope["path"])
    return route_path

[!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.

[amol-]

Based on my understanding of django/asgiref#229 (comment) @gabriel-f-santos raises a valid point. The comment states that the expectation when the ASGI specification was written was that web frameworks would strip the root_path themselves when performing routing.

This is what should be happening in https://github.com/encode/starlette/blob/master/starlette/_utils.py#L97-L98 (even though the check for "/" is confusing without a comment, as startswith was already checked previously)

[Kludex]

Gabriel's PR was merged...

[amol-]

Thanks for confirming, I just wanted to add a reference to the asgi specs discussion to confirm it's the intended behavior.

Also referencing here encode/uvicorn#2490 (comment) which causes unexpected duplication of root_path even if starlette removes it

[Kludex]

It's a bit hard to follow the root_path discussions... And the topic was discussed many times... If you are bringing a new point that was not covered before (which is hard to believe given the number of times this was revisited), it would be more useful to provide code examples, and comparison with other frameworks.

[amol-]

@Kludex , my comment and link to uvcorn is an attempt to make it more clear the behaviours (expected and implemented) to anyone finding this issue. As you mentioned it's a bit confusing the situation with root_path and the same questions have been brought up many times. I thought about adding the information as it would have been helpful for me when I was researching the issue

[Kludex]

Ah ok! Thanks!