From fe0dbd6874b995a07e05b308edcc782e1415d5aa Mon Sep 17 00:00:00 2001 From: Palmer Cox Date: Sun, 29 Sep 2024 12:23:34 -0400 Subject: [PATCH] Make handling of 'root_path' in dispatcher middleware adhere to ASGI spec Before this change, the dispatcher middleware didn't do anything with 'root_path'. It did, however, modify 'path'. The problem with this behavior, is that the child ASGI app has no way to determine what its prefix it. And if it can't determine its prefix, it doesn't know how to construct URLs. The ASGI spec isn't super clear on the expected behavior. However, some resources to review are: * https://asgi.readthedocs.io/en/latest/specs/www.html#wsgi-compatibility * https://github.com/encode/starlette/pull/2400 * https://github.com/django/asgiref/issues/229 Based on the above, I believe that the correct behavior is that "root_path" should be updated by the dispatcher middleware but that "path" should not be modified. In addition to the above change, I also updated the tests. And I also added a new test case where the dispatcher middleware is nested inside of itself. --- src/hypercorn/middleware/dispatcher.py | 5 +++-- tests/middleware/test_dispatcher.py | 23 +++++++++++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/hypercorn/middleware/dispatcher.py b/src/hypercorn/middleware/dispatcher.py index 427e26ac..13d2aa7d 100644 --- a/src/hypercorn/middleware/dispatcher.py +++ b/src/hypercorn/middleware/dispatcher.py @@ -18,9 +18,10 @@ async def __call__(self, scope: Scope, receive: Callable, send: Callable) -> Non if scope["type"] == "lifespan": await self._handle_lifespan(scope, receive, send) else: + relative_path = scope["path"][len(scope["root_path"]):] for path, app in self.mounts.items(): - if scope["path"].startswith(path): - scope["path"] = scope["path"][len(path) :] or "/" + if relative_path.startswith(path): + scope["root_path"] += path return await app(scope, receive, send) await send( { diff --git a/tests/middleware/test_dispatcher.py b/tests/middleware/test_dispatcher.py index 2d0b9e37..b0ecf86e 100644 --- a/tests/middleware/test_dispatcher.py +++ b/tests/middleware/test_dispatcher.py @@ -16,7 +16,7 @@ def __init__(self, name: str) -> None: async def __call__(self, scope: Scope, receive: Callable, send: Callable) -> None: scope = cast(HTTPScope, scope) - response = f"{self.name}-{scope['path']}" + response = f"{self.name}-{scope['path']}-{scope['root_path']}" await send( { "type": "http.response.start", @@ -27,7 +27,15 @@ async def __call__(self, scope: Scope, receive: Callable, send: Callable) -> Non await send({"type": "http.response.body", "body": response.encode()}) app = AsyncioDispatcherMiddleware( - {"/api/x": EchoFramework("apix"), "/api": EchoFramework("api")} + { + "/api/x": EchoFramework("apix"), + "/api/nested": AsyncioDispatcherMiddleware( + { + "/path": EchoFramework("nested"), + } + ), + "/api": EchoFramework("api"), + } ) sent_events = [] @@ -38,12 +46,15 @@ async def send(message: dict) -> None: await app({**http_scope, **{"path": "/api/x/b"}}, None, send) # type: ignore await app({**http_scope, **{"path": "/api/b"}}, None, send) # type: ignore + await app({**http_scope, **{"path": "/api/nested/path/x"}}, None, send) # type: ignore await app({**http_scope, **{"path": "/"}}, None, send) # type: ignore assert sent_events == [ - {"type": "http.response.start", "status": 200, "headers": [(b"content-length", b"7")]}, - {"type": "http.response.body", "body": b"apix-/b"}, - {"type": "http.response.start", "status": 200, "headers": [(b"content-length", b"6")]}, - {"type": "http.response.body", "body": b"api-/b"}, + {"type": "http.response.start", "status": 200, "headers": [(b"content-length", b"20")]}, + {"type": "http.response.body", "body": b"apix-/api/x/b-/api/x"}, + {"type": "http.response.start", "status": 200, "headers": [(b"content-length", b"15")]}, + {"type": "http.response.body", "body": b"api-/api/b-/api"}, + {"type": "http.response.start", "status": 200, "headers": [(b"content-length", b"42")]}, + {"type": "http.response.body", "body": b"nested-/api/nested/path/x-/api/nested/path"}, {"type": "http.response.start", "status": 404, "headers": [(b"content-length", b"0")]}, {"type": "http.response.body"}, ]