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', [