Giter Club home page Giter Club logo

Comments (6)

peterschutt avatar peterschutt commented on July 19, 2024

Also worth noting that if the path isn't dynamic, it is not a problem - this is b/c we handle "plain" routes separately, and before we test for mounted paths:

if path in plain_routes:
asgi_app, handler = parse_node_handlers(node=root_node.children[path], method=method)
return asgi_app, handler, path, {}

We then test for mounted paths before we step through the route trie:

if mount_paths_regex and (match := mount_paths_regex.match(path)):
mount_path = path[: match.end()]
mount_node = mount_routes[mount_path]
remaining_path = path[match.end() :]
# since we allow regular handlers under static paths, we must validate that the request does not match
# any such handler.
children = (
normalize_path(sub_route)
for sub_route in mount_node.children or []
if sub_route != mount_path and isinstance(sub_route, str)
)
if not any(remaining_path.startswith(f"{sub_route}/") for sub_route in children):
asgi_app, handler = parse_node_handlers(node=mount_node, method=method)
remaining_path = remaining_path or "/"
if not mount_node.is_static:
remaining_path = remaining_path if remaining_path.endswith("/") else f"{remaining_path}/"
return asgi_app, handler, remaining_path, {}

This is the source of the issue, b/c the dynamic route is only resolved as part of the trie traversal, but it never gets that far because the path /whatever/123 matches the mount path /.

So, I think we'd either need to get smarter with our mount_paths_regex so that it doesn't match on routes that are registered adjacent to the mounted application with variable paths, or, traverse the routing trie before checking for a match on a mount path, and only then try to match a mount path if we'd have otherwise thrown the 404.

from litestar.

peterschutt avatar peterschutt commented on July 19, 2024

Loosely related, we also cannot mount under a variable path, b/c the mount path regex includes the path variable placeholder, i.e., re.compile("/mounted/{num:int}") doesn't match /mounted/123.

E.g.,

@asgi("/mounted/{num:int}", is_mount=True)
async def mounted(scope: Scope, receive: Receive, send: Send) -> None:
    await send({"type": "http.response.start", "status": 200})
    await send({"type": "http.response.body", "body": b"Mounted!"})

from litestar.

peterschutt avatar peterschutt commented on July 19, 2024

Maybe mounts should be routed during tree traversal, instead of separate from it. If we hit a node that has both a mount and child nodes with regular handlers, we try to route it further via the trie, and if that fails we then pipe it through the mount?

from litestar.

ryanolf-tb avatar ryanolf-tb commented on July 19, 2024

I also came across this bug. Was going to file over at sqladmin-litestar-plugin but found it's already been raised there and pushed over here.

from litestar.

provinzkraut avatar provinzkraut commented on July 19, 2024

Maybe mounts should be routed during tree traversal, instead of separate from it. If we hit a node that has both a mount and child nodes with regular handlers, we try to route it further via the trie, and if that fails we then pipe it through the mount?

Sounds solid

from litestar.

provinzkraut avatar provinzkraut commented on July 19, 2024

Going back to this, I'm having second thought if we actually want to allow this. Mounting ASGI apps at the root path is always going to be kinda weird and cause unexpected behaviour in some cases, because you then have two entry points for your application. Maybe we should just not allow this in the first place and save us a lot of trouble?

from litestar.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.