Skip to content

Commit

Permalink
[PR #9852/249855a backport][3.11] Fix system routes polluting the mid…
Browse files Browse the repository at this point in the history
…dleware cache (#9856)
  • Loading branch information
bdraco authored Nov 13, 2024
1 parent d24c19e commit 2fa8bcd
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGES/9852.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed system routes polluting the middleware cache -- by :user:`bdraco`.
16 changes: 12 additions & 4 deletions aiohttp/web_app.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -54,6 +54,7 @@
MaskDomain,
MatchedSubAppResource,
PrefixedSubAppResource,
SystemRoute,
UrlDispatcher,
)

Expand All @@ -79,7 +80,6 @@
_Resource = TypeVar("_Resource", bound=AbstractResource)


@cache
def _build_middlewares(
handler: Handler, apps: Tuple["Application", ...]
) -> Callable[[Request], Awaitable[StreamResponse]]:
Expand All @@ -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(
[
Expand Down Expand Up @@ -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]
Expand Down
29 changes: 27 additions & 2 deletions tests/test_web_middleware.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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

0 comments on commit 2fa8bcd

Please sign in to comment.