Issue 12144: cookielib.CookieJar.make_cookies fails for cookies with 'expires' set

When cookielib.CookieJar().make_cookies is used to extract cookies from a urllib2 response, it crashes when it encounters a 'Set-Cookie' header entry that has an 'expires' attribute.

This crash occurs because the expires time is evaluated against the '_now' attribute of the CookieJar instance -- an attribute which is not set unless CookieJar().extract_cookies() was called previously.

Attached is a script that triggers this bug.

The actual error is triggered by line 1507 in '_cookie_from_cookie_tuple()'.

An easy fix is to move the setting of '_now' on line 1636 into the 'make_cookies()' method.

That addresses this problem and doesn't look like it would introduce any negative side effects.

Forgot to include the patch. Oops.

Could you possibly test the bug in Python 2.7, 3.1, 3.2 and current 3.3 branch?.

Python 2.6 is open for security fixes only, I think.

Exceptions with traceback are ordinary behavior issues. 'Crash' means segfault or equivalent on Windows. And Jesus is correct.

In general, include system with reports.
With 3.2.0 IDLE on Winxp, adjusted 3.x code

import urllib.request as ur, http.cookiejar as ck
cookie_jar = ck.CookieJar()
request = ur.Request('http://gdyn.cnn.com/1.1/1.gif?1301540335193')
conn = ur.urlopen(request)
cookie_jar.make_cookies(conn, request)

produces essentially same traceback ending in AttributeError.
I did not try the patch.

I was able to repro this with Terry's steps on latest hg update. I've taken Scott's patch and updated it to diff from source root (his was pointing to /usr/lib) against the latest. The patch fixes the issue and I also can't see any negative knock-ons that may be caused by applying it.

Can someone review the patch please.

As I marked in the Stage setting 3 yrs ago, the patch needs a test, in particular a acceptible unittest. I doubt that cnn.com qualifies. Senthil? David?

Perhaps we should have a test.python.org for use by tests, with oscure urls that return just what is needed for tests.

In 3.x, a new test should go in test_http_cookiejar.py.

This issue is still reproducible on master and below is a unittest. The patch looks reasonable to me and fixes the issue. @demian.brecht, would you like to convert the patch to a PR ?

diff --git a/Lib/test/test_http_cookiejar.py b/Lib/test/test_http_cookiejar.py
index 22bf41cf1d..3540a3d94f 100644
--- a/Lib/test/test_http_cookiejar.py
+++ b/Lib/test/test_http_cookiejar.py
@@ -585,6 +585,13 @@ class CookieTests(unittest.TestCase):
         # if expires is in future, keep cookie...
         c = CookieJar()
         future = time2netscape(time.time()+3600)
+
+        headers = ["Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; expires={0}".format(future)]
+        req = urllib.request.Request("http://www.coyote.com/")
+        res = FakeResponse(headers, "http://www.coyote.com/")
+        cookies = c.make_cookies(res, req)
+
+        c = CookieJar()
         interact_netscape(c, "http://www.acme.com/", 'spam="bar"; expires=%s' %
                           future)
         self.assertEqual(len(c), 1)

$ ./python.exe -m unittest -v test.test_http_cookiejar.CookieTests.test_expires
test_expires (test.test_http_cookiejar.CookieTests) ... /Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py:1619: UserWarning: http.cookiejar bug!
Traceback (most recent call last):
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py", line 1616, in make_cookies
    ns_cookies = self._cookies_from_attrs_set(
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py", line 1574, in _cookies_from_attrs_set
    cookie = self._cookie_from_cookie_tuple(tup, request)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py", line 1546, in _cookie_from_cookie_tuple
    elif expires <= self._now:
AttributeError: 'CookieJar' object has no attribute '_now'

  _warn_unhandled_exception()
ok

----------------------------------------------------------------------
Ran 1 test in 0.043s

OK

@xtreak sure, can do. May not have time to do so today but should be able to do so over the next couple days.

Karthikeyan, it looks like your test will pass even when the bug is not fixed. A test calling code that writes error message does not necessarily mean the test itself will fail, I don’t think.

I suggest you look at raising an exception when the UserWarning is triggered, and/or check that the expected cookie is returned with the right “expires” value.

> Karthikeyan, it looks like your test will pass even when the bug is not fixed. A test calling code that writes error message does not necessarily mean the test itself will fail, I don’t think.

You are right. Sorry, I got mislead by the Exception message and didn't notice the test was passing. The below patch to master ensures the test passes by asserting expires in the cookie. If @demian.brecht haven't had a chance to make a PR then I can try converting the to a PR adding them as co-author.

diff --git a/Lib/http/cookiejar.py b/Lib/http/cookiejar.py
index db82382357..07105a7c20 100644
--- a/Lib/http/cookiejar.py
+++ b/Lib/http/cookiejar.py
@@ -1590,6 +1590,7 @@ class CookieJar:
     def make_cookies(self, response, request):
         """Return sequence of Cookie objects extracted from response object."""
         # get cookie-attributes for RFC 2965 and Netscape protocols
+        self._policy._now = self._now = int(time.time())
         headers = response.info()
         rfc2965_hdrs = headers.get_all("Set-Cookie2", [])
         ns_hdrs = headers.get_all("Set-Cookie", [])
@@ -1672,8 +1673,6 @@ class CookieJar:
         _debug("extract_cookies: %s", response.info())
         self._cookies_lock.acquire()
         try:
-            self._policy._now = self._now = int(time.time())
-
             for cookie in self.make_cookies(response, request):
                 if self._policy.set_ok(cookie, request):
                     _debug(" setting cookie: %s", cookie)
diff --git a/Lib/test/test_http_cookiejar.py b/Lib/test/test_http_cookiejar.py
index 22bf41cf1d..ad3364c950 100644
--- a/Lib/test/test_http_cookiejar.py
+++ b/Lib/test/test_http_cookiejar.py
@@ -585,6 +585,14 @@ class CookieTests(unittest.TestCase):
         # if expires is in future, keep cookie...
         c = CookieJar()
         future = time2netscape(time.time()+3600)
+
+        headers = ["Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; expires={0}".format(future)]
+        req = urllib.request.Request("http://www.coyote.com/")
+        res = FakeResponse(headers, "http://www.coyote.com/")
+        cookies = c.make_cookies(res, req)
+        self.assertEqual(len(cookies), 1)
+        self.assertEqual(time2netscape(cookies[0].expires), future)
+
         interact_netscape(c, "http://www.acme.com/", 'spam="bar"; expires=%s' %
                           future)
         self.assertEqual(len(c), 1)

Failure without patch : 

./python.exe -m unittest -v test.test_http_cookiejar.CookieTests.test_expires
test_expires (test.test_http_cookiejar.CookieTests) ... /Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py:1619: UserWarning: http.cookiejar bug!
Traceback (most recent call last):
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py", line 1616, in make_cookies
    ns_cookies = self._cookies_from_attrs_set(
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py", line 1574, in _cookies_from_attrs_set
    cookie = self._cookie_from_cookie_tuple(tup, request)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/http/cookiejar.py", line 1546, in _cookie_from_cookie_tuple
    elif expires <= self._now:
AttributeError: 'CookieJar' object has no attribute '_now'

  _warn_unhandled_exception()
FAIL

======================================================================
FAIL: test_expires (test.test_http_cookiejar.CookieTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/test/test_http_cookiejar.py", line 593, in test_expires
    self.assertEqual(len(cookies), 1)
AssertionError: 0 != 1

----------------------------------------------------------------------
Ran 1 test in 0.017s

FAILED (failures=1)

New changeset bb41147eab15a2958f4ad38261e5bf608f6ace1b by Miss Islington (bot) (Xtreak) in branch 'master':
bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921)
https://github.com/python/cpython/commit/bb41147eab15a2958f4ad38261e5bf608f6ace1b

New changeset 44cb89a78a308b7a613bdd01539ec84be914d693 by Miss Islington (bot) in branch '3.8':
bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921)
https://github.com/python/cpython/commit/44cb89a78a308b7a613bdd01539ec84be914d693

New changeset e7b7edf5ebaed14dc68c841a8a98260f1330ef9a by Andrew Svetlov (Xtreak) in branch '3.7':
 [3.7] bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921) (GH-16092)
https://github.com/python/cpython/commit/e7b7edf5ebaed14dc68c841a8a98260f1330ef9a

messages: + msg338277
versions: + Python 3.7, Python 3.8, - Python 3.4, Python 3.5

messages: + msg220882
versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3

messages: + msg183105

messages: + msg137151

type: crash -> behavior
stage: test needed