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

Bug: mounted app path interferes with regular paths #3501

Closed
1 of 4 tasks
0xE111 opened this issue May 16, 2024 · 5 comments · Fixed by #3511
Closed
1 of 4 tasks

Bug: mounted app path interferes with regular paths #3501

0xE111 opened this issue May 16, 2024 · 5 comments · Fixed by #3511
Assignees
Labels
area/router Bug 🐛 This is something that is not working as expected

Comments

@0xE111
Copy link
Contributor

0xE111 commented May 16, 2024

Description

According to "Mounting ASGI apps" documentation section, Litestar can mount ASGI apps in sub-paths. So it is expected that if ASGI app is mounted with path='/magic', every route starting with /magic will be handled by the ASGI app, and any other route will be handled by other handlers. However, it is not true.

Imagine this setup:

@asgi("/magic", is_mount=True)
async def xxx(...):
    print('Mounted')
    ...

@get("/{number:int}/magic/")
async def yyy() -> str:
    print('Parametrized')

@get("/static/magic/")
async def zzz() -> str:
    print('Static')

Here's "expectations VS reality" table:

Request path Expected output Real output
/magic Mounted Mounted
/123/magic/ Parametrized Mounted
/static/magic/ Static Static
/non-existent/magic/ 404 error Mounted

Why this happens?

litestar/_asgi/routing_trie/traversal.py:parse_path_to_route method has this line:

if mount_paths_regex and (match := mount_paths_regex.search(path)):

So instead of matching /magic to path, re.search is used which searches for occurrence of /magic anywhere in path, thus resulting in "false positives" for strings such as /123/magic/, /non-existent/magic/ and /non-existent/magic/something.

Possible solution

This cannot be solved by simply using regex:

@asgi("^/magic", is_mount=True)

since mount_paths_regex becomes re.compile('/^/magic'), so it not only doesn't solve the problem, but the /magic endpoint itself stops working.

I believe it may be solved by replacing mount_paths_regex.search(path) with mount_paths_regex.match(path) - I did manual tests and it solved the problem completely, but ofc such a change requires tests to ensure nothing else is broken.

I am ready to create a full-fledged pull request with tests once the issue is approved :)

URL to code causing the issue

No response

MCVE

from typing import Any

from litestar import Litestar, asgi, get
from litestar.response.base import ASGIResponse


@asgi("/magic", is_mount=True)
async def mounted_handler(scope: Any, receive: Any, send: Any) -> None:
    body = 'mounted!'
    response = ASGIResponse(body=body.encode("utf-8"))
    await response(scope, receive, send)


@get("/{number:int}/magic/")
async def parametrized_handler() -> str:
    return 'parametrized'


@get("/static/magic/")
async def static_handler() -> str:
    return 'static'


app = Litestar(route_handlers=[
    mounted_handler,
    parametrized_handler,
    static_handler,
])

Steps to reproduce

1. Use the source code from MCVE to run Litestar app
2. Run curl to see wrong handler invoked for parametrized path:
  > curl http://127.0.0.1:8000/123/magic
  mounted!     
3. Run curl to see wrong handler invoked for non-existent path:
  > curl http://127.0.0.1:8000/whatever/magic
  mounted!

Screenshots

No response

Logs

No response

Litestar Version

2.8.3

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@0xE111 0xE111 added the Bug 🐛 This is something that is not working as expected label May 16, 2024
@peterschutt
Copy link
Contributor

Issue confirmed @0xE111 - I've assigned the issue to you. Thanks!

@peterschutt
Copy link
Contributor

@all-contributors add @0xE111 for bug

Copy link
Contributor

@peterschutt

I've put up a pull request to add @0xE111! 🎉

@provinzkraut
Copy link
Member

Thanks @0xE111 for the very detailed report!

I believe it may be solved by replacing mount_paths_regex.search(path) with mount_paths_regex.match(path) - I did manual tests and it solved the problem completely, but ofc such a change requires tests to ensure nothing else is broken.

This sounds like the correct fix here to get the intended behaviour. A PR would be very welcome :)

Copy link

github-actions bot commented Jun 2, 2024

A fix for this issue has been released in v2.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/router Bug 🐛 This is something that is not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants