From abcb2cc420bd8ca8a35e18876e05d260d010177c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 28 May 2025 13:38:17 -0500 Subject: [PATCH 1/5] Fix failing lint jobs due to caching (#11055) --- .github/workflows/ci-cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 80514363740..90e91228675 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') }}-v2 path: ~/.cache/pip restore-keys: | pip-lint- From 5925957204a641d1eb37c01f25ec2ba0c03226b5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 28 May 2025 14:36:09 -0500 Subject: [PATCH 2/5] Fix failing linter CI (#11060) --- .github/workflows/ci-cd.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 90e91228675..52888451b07 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') }}-v2 + key: pip-lint-${{ hashFiles('requirements/*.txt') }}-v3 path: ~/.cache/pip restore-keys: | pip-lint- @@ -79,6 +79,7 @@ jobs: slotscheck -v -m aiohttp - name: Install libenchant run: | + sudo apt-get update sudo apt install libenchant-2-dev - name: Install spell checker run: | From 7f691674ba44ff87308e6bf8e52c3c55c6ae43ab Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 28 May 2025 15:07:03 -0500 Subject: [PATCH 3/5] Prevent blockbuster False Positives from coverage.py Locking (#11056) --- tests/conftest.py | 8 ++++++++ 1 file changed, 8 insertions(+) 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 From e2eb1959237c8bc46a3f0e79a77c2086145d98cf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 28 May 2025 15:48:56 -0500 Subject: [PATCH 4/5] Fix CookieJar memory leak in filter_cookies() (#11054) --- CHANGES/11052.bugfix.rst | 2 ++ CHANGES/11054.bugfix.rst | 1 + aiohttp/cookiejar.py | 2 ++ tests/test_cookiejar.py | 54 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+) create mode 100644 CHANGES/11052.bugfix.rst create mode 120000 CHANGES/11054.bugfix.rst 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/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}" From 876102c111f10c2f4b23110d549db1c25e851b55 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 28 May 2025 16:17:24 -0500 Subject: [PATCH 5/5] Remove update of libenchant from linter workflow (#11064) --- .github/workflows/ci-cd.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 52888451b07..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') }}-v3 + key: pip-lint-${{ hashFiles('requirements/*.txt') }}-v4 path: ~/.cache/pip restore-keys: | pip-lint- @@ -77,10 +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-get update - sudo apt install libenchant-2-dev - name: Install spell checker run: | pip install -r requirements/doc-spelling.in -c requirements/doc-spelling.txt