From 25fca3101c48f18fcfa554c15277700b9a7581d6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 2 Nov 2023 22:44:52 -0500 Subject: [PATCH 1/6] Fix duplicate cookie expiration call in filter_cookies --- aiohttp/cookiejar.py | 1 - tests/test_cookiejar.py | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index a35c15f344b..dac95a4698a 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -234,7 +234,6 @@ def filter_cookies( self, request_url: URL = URL() ) -> Union["BaseCookie[str]", "SimpleCookie[str]"]: """Returns this jar's cookies filtered by their attributes.""" - self._do_expiration() if not isinstance(request_url, URL): warnings.warn( "The method accepts yarl.URL instances only, got {}".format( diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 00a32708756..5f4026015a6 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -809,6 +809,32 @@ async def test_cookie_jar_clear_expired(): assert len(sut) == 0 +async def test_cookie_jar_filter_cookies_expires(): + """Test that calling filter_cookies will expire stale cookies.""" + jar = CookieJar() + + # We do not iterate jar because it calls + # _do_expiration which will expire the cookie + # and invalidate the point of this test. + def _count_cookies(): + return sum(len(cookie.values()) for cookie in jar._cookies.values()) + + assert _count_cookies() == 0 + + with travel("1980-01-01", tick=False): + cookie = SimpleCookie() + cookie["foo"] = "bar" + cookie["foo"]["expires"] = "Tue, 1 Jan 1990 12:00:00 GMT" + jar.update_cookies(cookie) + + assert _count_cookies() == 1 + + # filter_cookies should expire stale cookies + jar.filter_cookies(URL("http://any.com/")) + + assert _count_cookies() == 0 + + async def test_cookie_jar_clear_domain() -> None: sut = CookieJar() cookie = SimpleCookie() From c1682c7c32e261b4da9682bee71b2f29c6262901 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 2 Nov 2023 22:44:52 -0500 Subject: [PATCH 2/6] timeline --- CHANGES/7784.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/7784.bugfix diff --git a/CHANGES/7784.bugfix b/CHANGES/7784.bugfix new file mode 100644 index 00000000000..ff7bb73702a --- /dev/null +++ b/CHANGES/7784.bugfix @@ -0,0 +1 @@ +Fix duplicate cookie expiration call in filter_cookies From fedaec352f17db39e864c1fda2c92482873d3c95 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 2 Nov 2023 23:06:34 -0500 Subject: [PATCH 3/6] len was checking for expire as well --- aiohttp/cookiejar.py | 7 ++++++- tests/test_cookiejar.py | 13 +++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index dac95a4698a..aee5fae946a 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -142,7 +142,12 @@ def __iter__(self) -> "Iterator[Morsel[str]]": yield from val.values() def __len__(self) -> int: - return sum(1 for i in self) + """Return number of cookies. + + This function does not iterate self to avoid unnecessary expiration + checks. + """ + return sum(len(cookie.values()) for cookie in self._cookies.values()) def _do_expiration(self) -> None: self.clear(lambda x: False) diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 5f4026015a6..322c99ce782 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -812,14 +812,7 @@ async def test_cookie_jar_clear_expired(): async def test_cookie_jar_filter_cookies_expires(): """Test that calling filter_cookies will expire stale cookies.""" jar = CookieJar() - - # We do not iterate jar because it calls - # _do_expiration which will expire the cookie - # and invalidate the point of this test. - def _count_cookies(): - return sum(len(cookie.values()) for cookie in jar._cookies.values()) - - assert _count_cookies() == 0 + assert len(jar) == 0 with travel("1980-01-01", tick=False): cookie = SimpleCookie() @@ -827,12 +820,12 @@ def _count_cookies(): cookie["foo"]["expires"] = "Tue, 1 Jan 1990 12:00:00 GMT" jar.update_cookies(cookie) - assert _count_cookies() == 1 + assert len(jar) == 1 # filter_cookies should expire stale cookies jar.filter_cookies(URL("http://any.com/")) - assert _count_cookies() == 0 + assert len(jar) == 0 async def test_cookie_jar_clear_domain() -> None: From 775392136a72dace425801ced8573a6781e8f9fa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 2 Nov 2023 23:08:49 -0500 Subject: [PATCH 4/6] reword --- CHANGES/7784.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/7784.bugfix b/CHANGES/7784.bugfix index ff7bb73702a..1f8ba8ddb44 100644 --- a/CHANGES/7784.bugfix +++ b/CHANGES/7784.bugfix @@ -1 +1 @@ -Fix duplicate cookie expiration call in filter_cookies +Fix duplicate cookie expiration calls in the CookieJar implementation From 5b0866da5135e42b9a3280314233378cccc2c171 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 2 Nov 2023 23:32:34 -0500 Subject: [PATCH 5/6] cpython only --- tests/test_cookiejar.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 322c99ce782..407f7b92288 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -809,6 +809,10 @@ async def test_cookie_jar_clear_expired(): assert len(sut) == 0 +@pytest.mark.skipif( + sys.implementation.name != "cpython", + reason="time_machine leverages CPython specific pointers https://github.com/adamchainz/time-machine/issues/305", +) async def test_cookie_jar_filter_cookies_expires(): """Test that calling filter_cookies will expire stale cookies.""" jar = CookieJar() From 5139814310c0c282143ebbd198f1446d3a881060 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 20 Jan 2024 14:25:42 -1000 Subject: [PATCH 6/6] update test --- tests/test_cookiejar.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 4f0eda7cd8c..6ff00bf41a6 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -864,19 +864,17 @@ async def test_cookie_jar_clear_expired(): assert len(sut) == 0 -@pytest.mark.skipif( - sys.implementation.name != "cpython", - reason="time_machine leverages CPython specific pointers https://github.com/adamchainz/time-machine/issues/305", -) async def test_cookie_jar_filter_cookies_expires(): """Test that calling filter_cookies will expire stale cookies.""" jar = CookieJar() assert len(jar) == 0 - with travel("1980-01-01", tick=False): - cookie = SimpleCookie() - cookie["foo"] = "bar" - cookie["foo"]["expires"] = "Tue, 1 Jan 1990 12:00:00 GMT" + cookie = SimpleCookie() + + cookie["foo"] = "bar" + cookie["foo"]["expires"] = "Tue, 1 Jan 1990 12:00:00 GMT" + + with freeze_time("1980-01-01"): jar.update_cookies(cookie) assert len(jar) == 1