diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 80514363740..da2bcb9f6c4 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -53,7 +53,7 @@ jobs: - name: Cache PyPI uses: actions/cache@v4.2.3 with: - key: pip-lint-${{ hashFiles('requirements/*.txt') }} + key: pip-lint-${{ hashFiles('requirements/*.txt') }}-v4 path: ~/.cache/pip restore-keys: | pip-lint- @@ -77,9 +77,6 @@ jobs: # can be scanned by slotscheck. pip install -r requirements/base.in -c requirements/base.txt slotscheck -v -m aiohttp - - name: Install libenchant - run: | - sudo apt install libenchant-2-dev - name: Install spell checker run: | pip install -r requirements/doc-spelling.in -c requirements/doc-spelling.txt diff --git a/CHANGES/11052.bugfix.rst b/CHANGES/11052.bugfix.rst new file mode 100644 index 00000000000..73e4ea216c8 --- /dev/null +++ b/CHANGES/11052.bugfix.rst @@ -0,0 +1,2 @@ +Fixed memory leak in :py:meth:`~aiohttp.CookieJar.filter_cookies` that caused unbounded memory growth +when making requests to different URL paths -- by :user:`bdraco` and :user:`Cycloctane`. diff --git a/CHANGES/11054.bugfix.rst b/CHANGES/11054.bugfix.rst new file mode 120000 index 00000000000..2d6e2428f3e --- /dev/null +++ b/CHANGES/11054.bugfix.rst @@ -0,0 +1 @@ +11052.bugfix.rst \ No newline at end of file diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 158d6d103ab..1e286dffed2 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -354,6 +354,8 @@ def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": path_len = len(request_url.path) # Point 2: https://www.rfc-editor.org/rfc/rfc6265.html#section-5.4 for p in pairs: + if p not in self._cookies: + continue for name, cookie in self._cookies[p].items(): domain = cookie["domain"] diff --git a/tests/conftest.py b/tests/conftest.py index 17b51266cd7..62fb04f2e36 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -81,6 +81,14 @@ def blockbuster(request: pytest.FixtureRequest) -> Iterator[None]: bb.functions[func].can_block_in( "aiohttp/web_urldispatcher.py", "add_static" ) + # Note: coverage.py uses locking internally which can cause false positives + # in blockbuster when it instruments code. This is particularly problematic + # on Windows where it can lead to flaky test failures. + # Additionally, we're not particularly worried about threading.Lock.acquire happening + # by accident in this codebase as we primarily use asyncio.Lock for + # synchronization in async code. + # Allow lock.acquire calls to prevent these false positives + bb.functions["threading.Lock.acquire"].deactivate() yield diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 1f78c5abba9..364b8a701c5 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -1151,3 +1151,57 @@ async def test_treat_as_secure_origin() -> None: assert len(jar) == 1 filtered_cookies = jar.filter_cookies(request_url=endpoint) assert len(filtered_cookies) == 1 + + +async def test_filter_cookies_does_not_leak_memory() -> None: + """Test that filter_cookies doesn't create empty cookie entries. + + Regression test for https://github.com/aio-libs/aiohttp/issues/11052 + """ + jar = CookieJar() + + # Set a cookie with Path=/ + jar.update_cookies({"test_cookie": "value; Path=/"}, URL("http://example.com/")) + + # Check initial state + assert len(jar) == 1 + initial_storage_size = len(jar._cookies) + initial_morsel_cache_size = len(jar._morsel_cache) + + # Make multiple requests with different paths + paths = [ + "/", + "/api", + "/api/v1", + "/api/v1/users", + "/api/v1/users/123", + "/static/css/style.css", + "/images/logo.png", + ] + + for path in paths: + url = URL(f"http://example.com{path}") + filtered = jar.filter_cookies(url) + # Should still get the cookie + assert len(filtered) == 1 + assert "test_cookie" in filtered + + # Storage size should not grow significantly + # Only the shared cookie entry ('', '') may be added + final_storage_size = len(jar._cookies) + assert final_storage_size <= initial_storage_size + 1 + + # Verify _morsel_cache doesn't leak either + # It should only have entries for domains/paths where cookies exist + final_morsel_cache_size = len(jar._morsel_cache) + assert final_morsel_cache_size <= initial_morsel_cache_size + 1 + + # Verify no empty entries were created for domain-path combinations + for key, cookies in jar._cookies.items(): + if key != ("", ""): # Skip the shared cookie entry + assert len(cookies) > 0, f"Empty cookie entry found for {key}" + + # Verify _morsel_cache entries correspond to actual cookies + for key, morsels in jar._morsel_cache.items(): + assert key in jar._cookies, f"Orphaned morsel cache entry for {key}" + assert len(morsels) > 0, f"Empty morsel cache entry found for {key}"