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: regular handler under asgi mount path conflicts with routing to asgi app #3429

Closed
1 of 4 tasks
peterschutt opened this issue Apr 26, 2024 · 3 comments · Fixed by #3430
Closed
1 of 4 tasks

Bug: regular handler under asgi mount path conflicts with routing to asgi app #3429

peterschutt opened this issue Apr 26, 2024 · 3 comments · Fixed by #3430
Labels
Bug 🐛 This is something that is not working as expected

Comments

@peterschutt
Copy link
Contributor

peterschutt commented Apr 26, 2024

Description

If we have a regular handler mounted under an asgi mounted path, and the path of the regular handler is a non-prefix sub-string of the a request path the request will not get routed to the asgi app.

I.e., if we have an asgi mounted on "/", and a regular handler at "/path", a request to "/some/path" does not get routed to the asgi app.

URL to code causing the issue

No response

MCVE

from __future__ import annotations

from typing import TYPE_CHECKING

from litestar import Litestar, asgi, get
from litestar.testing import TestClient

if TYPE_CHECKING:
    from litestar.types.asgi_types import Receive, Scope, Send


async def asgi_app(scope: Scope, receive: Receive, send: Send) -> None:
    assert scope["type"] == "http"
    await send({
        "type": "http.response.start",
        "status": 200,
        "headers": [
            (b"content-type", b"text/plain"),
            (b"content-length", b"%d" % len(scope["raw_path"])),
        ],
    })
    await send({
        "type": "http.response.body",
        "body": scope["raw_path"],
    })

asgi_handler = asgi("/", is_mount=True)(asgi_app)


@get("/path")
def get_handler() -> str:
    return "Hello, world!"


def test_regular_handler_under_mounted_asgi_app() -> None:
    app = Litestar(
        route_handlers=[asgi("/", is_mount=True)(asgi_app), get_handler],
        openapi_config=None,
        debug=True,
    )

    with TestClient(app) as client:
        resp = client.get("/some/path")  # currently this is a 404
        assert resp.content == b"/some/path"

Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

"![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

No response

Litestar Version

main

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
@peterschutt peterschutt added the Bug 🐛 This is something that is not working as expected label Apr 26, 2024
@peterschutt
Copy link
Contributor Author

Issue looks to come from here:

children = [sub_route for sub_route in mount_node.children or [] if sub_route != mount_path]
if not children or all(sub_route not in path for sub_route in children): # type: ignore[operator]

Where path == "/some/path" and children == ["/path"], the request isn't directed to the mounted application because the path contains the child path. I think instead of testing whether the sub-route is a substring of the path, we should check path.startswith(f"{sub_route}/").

Copy link

This issue has been closed in #3430. The change will be included in the upcoming patch release.

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
Bug 🐛 This is something that is not working as expected
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

1 participant