From 2fa8bcd6b5438b0cb7912b35ff853493bdaebc1b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Nov 2024 08:56:19 -0600 Subject: [PATCH] [PR #9852/249855a backport][3.11] Fix system routes polluting the middleware cache (#9856) --- CHANGES/9852.bugfix.rst | 1 + aiohttp/web_app.py | 16 ++++++++++++---- tests/test_web_middleware.py | 29 +++++++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 6 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 c29f32df413..5d542ab9222 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, @@ -54,6 +54,7 @@ MaskDomain, MatchedSubAppResource, PrefixedSubAppResource, + SystemRoute, UrlDispatcher, ) @@ -79,7 +80,6 @@ _Resource = TypeVar("_Resource", bound=AbstractResource) -@cache def _build_middlewares( handler: Handler, apps: Tuple["Application", ...] ) -> Callable[[Request], Awaitable[StreamResponse]]: @@ -90,6 +90,9 @@ def _build_middlewares( return handler +_cached_build_middleware = lru_cache(maxsize=1024)(_build_middlewares) + + class Application(MutableMapping[Union[str, AppKey[Any]], Any]): ATTRS = frozenset( [ @@ -544,8 +547,13 @@ async def _handle(self, request: Request) -> StreamResponse: handler = match_info.handler if self._run_middlewares: - if not self._has_legacy_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 not self._has_legacy_middlewares and not isinstance( + match_info.route, SystemRoute + ): + handler = _cached_build_middleware(handler, match_info.apps) else: for app in match_info.apps[::-1]: for m, new_style in app._middlewares_handlers: # type: ignore[union-attr] diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py index 9c4462be409..13acc589da9 100644 --- a/tests/test_web_middleware.py +++ b/tests/test_web_middleware.py @@ -1,10 +1,11 @@ import re -from typing import Any +from typing import Any, NoReturn import pytest from yarl import URL -from aiohttp import web +from aiohttp import web, web_app +from aiohttp.pytest_plugin import AiohttpClient from aiohttp.typedefs import Handler @@ -520,3 +521,27 @@ async def call(self, request, handler: Handler): 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: + @web.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