New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cookielib.CookieJar.make_cookies fails for cookies with 'expires' set #56353
Comments
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. 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 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. |
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 ====================================================================== 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) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: