From 5d82af810f8e97f7d7c50e54085fcf533377d274 Mon Sep 17 00:00:00 2001 From: Mikko Alutoin Date: Fri, 31 Jul 2026 11:22:45 +0300 Subject: [PATCH] Remove cookies with a non-positive `Max-Age` from sessions Completes the fix for #998, which reported that a cookie carrying only `Max-Age=0` (no `Expires`) was never removed from the session file. #1029 addressed that by translating `max-age` into `expires` in `_max_age_to_expires()`, gated on `max_age.isdigit()`. `str.isdigit()` is False for any negative value, so `Max-Age=-1` still never receives an `expires` key, is never recognised as expired by `get_expired_cookies()`, and is never removed -- the same symptom #998 described, for the other common deletion idiom. HTTPie keeps sending a cookie the server has told it to delete. RFC 6265 section 5.2.2: "If delta-seconds is less than or equal to zero, let expiry-time be the earliest representable date and time." Parse with `int()` instead, so negative deltas are honoured while unparseable values are still ignored rather than raising -- note that a narrower fix such as `max_age.lstrip('-').isdigit()` would make `int('--1')` throw, which the new test pins. This matches the standard library, which parses `max-age` with `int()` in `http.cookiejar`. Also rewrites the docstring. It read `HACK/FIXME: `, but that issue was declined as a no-op rather than fixed, so this is not a workaround awaiting upstream removal. The docstring now records the RFC rule, and that the function is only load-bearing for pre-3.1.0 session layouts: those stored cookies as a domainless dict, so `http.cookiejar`'s `clear(domain, path, name)` could not match them, whereas since 3.1.0 each cookie is bound to a domain (#1312) and the cookiejar removes expired ones by itself. Removing this function therefore means dropping support for the old layout first. Co-Authored-By: Claude Opus 5 --- httpie/utils.py | 32 ++++++++++++++++++++++++++++---- tests/test_sessions.py | 25 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/httpie/utils.py b/httpie/utils.py index 4735b2be5d..4fda6ed2f7 100644 --- a/httpie/utils.py +++ b/httpie/utils.py @@ -187,17 +187,41 @@ def is_expired(expires: Optional[float]) -> bool: def _max_age_to_expires(cookies, now): """ - Translate `max-age` into `expires` for Requests to take it into account. + Translate `max-age` into `expires` so an expired cookie is recognised as such. - HACK/FIXME: + Parsing follows RFC 6265 section 5.2.2: a `delta-seconds` of zero or less means the + cookie expires immediately, so negative values must be honoured and not just zero. + + Why HTTPie does this itself, and when it still matters: + + Originally none of it was handled downstream, which is what #998 reported. The + upstream report at was declined as a + no-op -- Requests considers the standard library's behaviour correct -- so no upstream + change is coming and this is not a workaround awaiting removal. + + It is, however, only load-bearing for session files in the pre-3.1.0 layout. Those + stored cookies as a dict with no per-cookie domain, and `http.cookiejar` deletes an + existing cookie via `clear(domain, path, name)`, so nothing matched and the cookie + survived. Since 3.1.0 the layout binds each cookie to a domain (#1312), so the + cookiejar removes expired cookies on its own -- for those sessions this function is + redundant. + + Dropping it therefore means dropping support for the pre-3.1.0 layout first; see + `httpie.legacy.v3_1_0_session_cookie_format`, which still reads it. """ for cookie in cookies: if 'expires' in cookie: continue max_age = cookie.get('max-age') - if max_age and max_age.isdigit(): - cookie['expires'] = now + float(max_age) + try: + # `str.isdigit()` would reject a negative delta-seconds, but RFC 6265 + # section 5.2.2 requires a value of zero or less to expire the cookie + # immediately — that is how servers delete cookies with `Max-Age=-1`. + delta_seconds = int(max_age) + except (TypeError, ValueError): + continue + cookie['expires'] = now + delta_seconds def parse_content_type_header(header): diff --git a/tests/test_sessions.py b/tests/test_sessions.py index aa5243487d..2ecf5c7c8a 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -401,6 +401,31 @@ def test_get_expired_cookies_using_max_age(self): ] assert get_expired_cookies(cookies, now=None) == expected_expired + @pytest.mark.parametrize( + 'max_age, expected_expired', + [ + # RFC 6265 5.2.2: "If delta-seconds is less than or equal to zero, + # let expiry-time be the earliest representable date and time." + ('0', True), + ('-1', True), + ('-100', True), + # Positive ages are in the future, so not expired. + ('3600', False), + # Unparseable values must be ignored rather than raising. + ('', False), + ('abc', False), + ('1.5', False), + ('--1', False), + ] + ) + def test_get_expired_cookies_using_non_positive_max_age(self, max_age, + expected_expired): + cookies = f'one=two; Max-Age={max_age}; path=/; domain=.tumblr.com' + expired = get_expired_cookies(cookies, now=None) + assert bool(expired) is expected_expired + if expected_expired: + assert expired == [{'name': 'one', 'path': '/'}] + @pytest.mark.parametrize( 'cookies, now, expected_expired', [