From fc895c4a8a42b17e99f30ee30606f21907515fd1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Nov 2024 07:58:08 -0600 Subject: [PATCH] Fix system routes polluting the middleware cache --- CHANGES/9852.bugfix.rst | 1 + aiohttp/web_app.py | 15 ++++++++++++--- tests/test_web_middleware.py | 25 ++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 CHANGES/9852.bugfix.rst diff --git a/CHANGES/9852.bugfix.rst b/CHANGES/9852.bugfix.rst new file mode 100644 index 00000000000..b459d08478b --- /dev/null +++ b/CHANGES/9852.bugfix.rst @@ -0,0 +1 @@ +Fixed system routes polluting the middleware cache -- by :user:`bdraco`. diff --git a/aiohttp/web_app.py b/aiohttp/web_app.py index 96b1b93cac0..78fb1da8514 100644 --- a/aiohttp/web_app.py +++ b/aiohttp/web_app.py @@ -1,7 +1,7 @@ import asyncio import logging import warnings -from functools import cache, partial, update_wrapper +from functools import lru_cache, partial, update_wrapper from typing import ( TYPE_CHECKING, Any, @@ -44,6 +44,7 @@ MaskDomain, MatchedSubAppResource, PrefixedSubAppResource, + SystemRoute, UrlDispatcher, ) @@ -70,7 +71,6 @@ _Resource = TypeVar("_Resource", bound=AbstractResource) -@cache def _build_middlewares( handler: Handler, apps: Tuple["Application", ...] ) -> Callable[[Request], Awaitable[StreamResponse]]: @@ -84,6 +84,9 @@ def _build_middlewares( return handler +_cached_build_middleware = lru_cache(maxsize=1024)(_build_middlewares) + + @final class Application(MutableMapping[Union[str, AppKey[Any]], Any]): __slots__ = ( @@ -397,7 +400,13 @@ async def _handle(self, request: Request) -> StreamResponse: handler = match_info.handler if self._run_middlewares: - handler = _build_middlewares(handler, match_info.apps) + # If its a SystemRoute, don't cache building the middlewares since + # they are constructed for every MatchInfoError as a new handler + # is made each time. + if isinstance(match_info.route, SystemRoute): + handler = _build_middlewares(handler, match_info.apps) + else: + handler = _cached_build_middleware(handler, match_info.apps) return await handler(request) diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py index 7a7b2c8e91a..5b8f1c78166 100644 --- a/tests/test_web_middleware.py +++ b/tests/test_web_middleware.py @@ -4,7 +4,7 @@ import pytest from yarl import URL -from aiohttp import web +from aiohttp import web, web_app from aiohttp.pytest_plugin import AiohttpClient from aiohttp.test_utils import TestClient from aiohttp.typedefs import Handler, Middleware @@ -522,3 +522,26 @@ async def call(self, request: web.Request, handler: Handler) -> web.Response: assert 201 == resp.status txt = await resp.text() assert "OK[new style middleware]" == txt + + +async def test_middleware_does_not_leak(aiohttp_client: AiohttpClient) -> None: + async def any_handler(request: web.Request) -> NoReturn: + assert False + + class Middleware: + async def call( + self, request: web.Request, handler: Handler + ) -> web.StreamResponse: + return await handler(request) + + app = web.Application() + app.router.add_route("POST", "/any", any_handler) + app.middlewares.append(Middleware().call) + + client = await aiohttp_client(app) + + web_app._cached_build_middleware.cache_clear() + for _ in range(10): + resp = await client.get("/any") + assert resp.status == 405 + assert web_app._cached_build_middleware.cache_info().currsize < 10