From 76cbba45b9c98832f2d3e2e6caf46e36b1090491 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Date: Sat, 13 Jun 2026 02:53:10 -0700 Subject: [PATCH 1/2] Make RequestsCookieJar.popitem() work RequestsCookieJar subclasses both CookieJar and MutableMapping. Its __iter__ comes from CookieJar and yields Cookie objects rather than names, so the popitem() inherited from MutableMapping did `self[next(iter(self))]`, i.e. looked a cookie up by a Cookie object, and always raised KeyError even when the jar was non-empty. Override popitem() to remove and return a (name, value) pair using the jar's dict-like view, and raise KeyError only when the jar is empty, matching the documented behavior. --- src/requests/cookies.py | 14 ++++++++++++++ tests/test_requests.py | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/requests/cookies.py b/src/requests/cookies.py index 24dd9a7829..b6987ab5ba 100644 --- a/src/requests/cookies.py +++ b/src/requests/cookies.py @@ -299,6 +299,20 @@ def items(self) -> list[tuple[str, str | None]]: # type: ignore[override] """ return list(self.iteritems()) + def popitem(self) -> tuple[str, str | None]: + """Dict-like popitem() that removes and returns a (name, value) pair, + or raises KeyError if the jar is empty. + + The inherited ``MutableMapping.popitem`` does not work here because + ``__iter__`` yields ``Cookie`` objects rather than names. + """ + try: + name, value = next(self.iteritems()) + except StopIteration: + raise KeyError("popitem(): cookie jar is empty") from None + del self[name] + return name, value + def list_domains(self) -> list[str]: """Utility method to list all the domains in the jar.""" domains: list[str] = [] diff --git a/tests/test_requests.py b/tests/test_requests.py index 8bcb81d8b1..794be0f63c 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -1313,6 +1313,24 @@ def test_cookie_parameters(self): assert cookie.domain == domain assert cookie._rest["HttpOnly"] == rest["HttpOnly"] + def test_cookie_popitem(self): + jar = requests.cookies.RequestsCookieJar() + jar.set("some_cookie", "some_value") + jar.set("some_cookie1", "some_value1") + + items = dict(jar.items()) + + name, value = jar.popitem() + assert (name, value) in items.items() + assert name not in jar + assert len(jar) == 1 + + jar.popitem() + assert len(jar) == 0 + + with pytest.raises(KeyError): + jar.popitem() + def test_cookie_as_dict_keeps_len(self): key = "some_cookie" value = "some_value" From 6aa944469926f4eba9c4f535b7fe4039370ceef3 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Date: Mon, 15 Jun 2026 23:41:08 -0700 Subject: [PATCH 2/2] Remove the exact cookie in popitem() instead of deleting by name popitem() read the first (name, value) pair from the iterator but then removed the cookie with `del self[name]`, which deletes every cookie sharing that name across other domains or paths. With two cookies of the same name on different domains, that dropped both even though popitem() reported removing only one. Select the cookie via the iterator and remove that exact object with clear(domain, path, name), so only the returned cookie is discarded. Add a regression test covering two same-named cookies on different domains. --- src/requests/cookies.py | 9 ++++++--- tests/test_requests.py | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/requests/cookies.py b/src/requests/cookies.py index b6987ab5ba..345d490327 100644 --- a/src/requests/cookies.py +++ b/src/requests/cookies.py @@ -307,11 +307,14 @@ def popitem(self) -> tuple[str, str | None]: ``__iter__`` yields ``Cookie`` objects rather than names. """ try: - name, value = next(self.iteritems()) + cookie = next(iter(self)) except StopIteration: raise KeyError("popitem(): cookie jar is empty") from None - del self[name] - return name, value + # Remove the exact cookie the iterator selected. Deleting by name alone + # (``del self[name]``) would drop every cookie sharing that name across + # other domains/paths, not just the one returned here. + self.clear(cookie.domain, cookie.path, cookie.name) + return cookie.name, cookie.value def list_domains(self) -> list[str]: """Utility method to list all the domains in the jar.""" diff --git a/tests/test_requests.py b/tests/test_requests.py index 794be0f63c..ac6398e8f2 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -1331,6 +1331,22 @@ def test_cookie_popitem(self): with pytest.raises(KeyError): jar.popitem() + def test_cookie_popitem_removes_only_the_selected_cookie(self): + # Two cookies share a name but live on different domains. popitem() must + # remove exactly the one it returns, not every cookie with that name. + jar = requests.cookies.RequestsCookieJar() + jar.set("dup", "value-a", domain="a.example.com", path="/") + jar.set("dup", "value-b", domain="b.example.com", path="/") + assert len(jar) == 2 + + name, value = jar.popitem() + assert name == "dup" + assert len(jar) == 1 + + remaining = next(iter(jar)) + assert remaining.name == "dup" + assert {value, remaining.value} == {"value-a", "value-b"} + def test_cookie_as_dict_keeps_len(self): key = "some_cookie" value = "some_value"