classification
Title: cookielib.CookieJar.make_cookies fails for cookies with 'expires' set
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Scott.Wimer, asvetlov, demian.brecht, jcea, martin.panter, miss-islington, orsenthil, r.david.murray, terry.reedy, xtreak
Priority: normal Keywords: patch

Created on 2011-05-22 02:58 by Scott.Wimer, last changed 2019-09-13 12:22 by asvetlov. This issue is now closed.

Files
File name Uploaded Description Edit
cookielib-crash.py Scott.Wimer, 2011-05-22 02:58 Trigger the bug.
cookielib-crash.patch Scott.Wimer, 2011-05-22 03:09 Patch to fix cookielib.py to handle expires header in make_cookies.
cookiejar_12144.patch demian.brecht, 2013-02-27 01:56 Update on latest source, from project root. review
Pull Requests
URL Status Linked Edit
PR 13921 merged xtreak, 2019-06-08 18:00
PR 16088 merged miss-islington, 2019-09-13 11:29
PR 16089 closed miss-islington, 2019-09-13 11:29
PR 16092 merged xtreak, 2019-09-13 12:02
Messages (15)
msg136497 - (view) Author: Scott Wimer (Scott.Wimer) Date: 2011-05-22 02:58
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.
msg136498 - (view) Author: Scott Wimer (Scott.Wimer) Date: 2011-05-22 03:07
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.
msg136499 - (view) Author: Scott Wimer (Scott.Wimer) Date: 2011-05-22 03:09
Forgot to include the patch. Oops.
msg136580 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2011-05-23 00:16
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.
msg137151 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-05-28 20:08
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.
msg183105 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2013-02-27 01:56
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.
msg220882 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-17 20:39
Can someone review the patch please.
msg220905 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-17 22:07
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.
msg338277 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-03-18 18:20
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
msg338281 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2019-03-18 19:17
@xtreak sure, can do. May not have time to do so today but should be able to do so over the next couple days.
msg340825 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2019-04-25 07:36
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.
msg340827 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-04-25 09:05
> 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)
msg352290 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 11:29
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
msg352299 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 11:47
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
msg352309 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-09-13 12:22
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
History
Date User Action Args
2019-09-13 12:22:28asvetlovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-09-13 12:22:15asvetlovsetnosy: + asvetlov
messages: + msg352309
2019-09-13 12:02:44xtreaksetpull_requests: + pull_request15713
2019-09-13 12:01:30asvetlovsetversions: + Python 3.9, - Python 2.7
2019-09-13 11:47:54miss-islingtonsetmessages: + msg352299
2019-09-13 11:29:18miss-islingtonsetpull_requests: + pull_request15710
2019-09-13 11:29:11miss-islingtonsetpull_requests: + pull_request15709
2019-09-13 11:29:03miss-islingtonsetnosy: + miss-islington
messages: + msg352290
2019-06-08 18:00:30xtreaksetstage: test needed -> patch review
pull_requests: + pull_request13794
2019-04-25 09:05:07xtreaksetmessages: + msg340827
2019-04-25 07:36:20martin.pantersetnosy: + martin.panter
messages: + msg340825
2019-03-18 19:17:38demian.brechtsetmessages: + msg338281
2019-03-18 18:20:43xtreaksetnosy: + xtreak

messages: + msg338277
versions: + Python 3.7, Python 3.8, - Python 3.4, Python 3.5
2019-03-15 23:24:54BreamoreBoysetnosy: - BreamoreBoy
2014-06-17 22:07:40terry.reedysetnosy: + orsenthil, r.david.murray
messages: + msg220905
2014-06-17 20:39:02BreamoreBoysetnosy: + BreamoreBoy

messages: + msg220882
versions: + Python 3.4, Python 3.5, - Python 3.2, Python 3.3
2013-02-27 01:56:14demian.brechtsetfiles: + cookiejar_12144.patch

messages: + msg183105
2013-02-27 01:39:22demian.brechtsetnosy: + demian.brecht
2011-05-28 20:08:16terry.reedysetversions: + Python 2.7, Python 3.2, Python 3.3, - Python 2.6
nosy: + terry.reedy

messages: + msg137151

type: crash -> behavior
stage: test needed
2011-05-23 00:16:22jceasetmessages: + msg136580
2011-05-23 00:14:11jceasetnosy: + jcea
2011-05-22 03:09:41Scott.Wimersetfiles: + cookielib-crash.patch
keywords: + patch
messages: + msg136499
2011-05-22 03:07:55Scott.Wimersetmessages: + msg136498
2011-05-22 02:58:44Scott.Wimercreate