Skip to content
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

Closed
ScottWimer mannequin opened this issue May 22, 2011 · 15 comments
Closed

cookielib.CookieJar.make_cookies fails for cookies with 'expires' set #56353

ScottWimer mannequin opened this issue May 22, 2011 · 15 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ScottWimer
Copy link
Mannequin

ScottWimer mannequin commented May 22, 2011

BPO 12144
Nosy @terryjreedy, @jcea, @orsenthil, @bitdancer, @asvetlov, @vadmium, @demianbrecht, @miss-islington, @tirkarthi
PRs
  • bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies #13921
  • [3.8] bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921) #16088
  • [3.7] bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921) #16089
  • [3.7] bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921) #16092
  • Files
  • cookielib-crash.py: Trigger the bug.
  • cookielib-crash.patch: Patch to fix cookielib.py to handle expires header in make_cookies.
  • cookiejar_12144.patch: Update on latest source, from project root.
  • 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:

    assignee = None
    closed_at = <Date 2019-09-13.12:22:28.942>
    created_at = <Date 2011-05-22.02:58:44.317>
    labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
    title = "cookielib.CookieJar.make_cookies fails for cookies with 'expires' set"
    updated_at = <Date 2019-09-13.12:22:28.941>
    user = 'https://bugs.python.org/ScottWimer'

    bugs.python.org fields:

    activity = <Date 2019-09-13.12:22:28.941>
    actor = 'asvetlov'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-09-13.12:22:28.942>
    closer = 'asvetlov'
    components = ['Library (Lib)']
    creation = <Date 2011-05-22.02:58:44.317>
    creator = 'Scott.Wimer'
    dependencies = []
    files = ['22054', '22055', '29257']
    hgrepos = []
    issue_num = 12144
    keywords = ['patch']
    message_count = 15.0
    messages = ['136497', '136498', '136499', '136580', '137151', '183105', '220882', '220905', '338277', '338281', '340825', '340827', '352290', '352299', '352309']
    nosy_count = 10.0
    nosy_names = ['terry.reedy', 'jcea', 'orsenthil', 'r.david.murray', 'asvetlov', 'Scott.Wimer', 'martin.panter', 'demian.brecht', 'miss-islington', 'xtreak']
    pr_nums = ['13921', '16088', '16089', '16092']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue12144'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @ScottWimer
    Copy link
    Mannequin Author

    ScottWimer mannequin commented May 22, 2011

    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.

    @ScottWimer ScottWimer mannequin added type-crash A hard crash of the interpreter, possibly with a core dump stdlib Python modules in the Lib dir labels May 22, 2011
    @ScottWimer
    Copy link
    Mannequin Author

    ScottWimer mannequin commented May 22, 2011

    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.

    @ScottWimer
    Copy link
    Mannequin Author

    ScottWimer mannequin commented May 22, 2011

    Forgot to include the patch. Oops.

    @jcea
    Copy link
    Member

    jcea commented May 23, 2011

    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.

    @terryjreedy
    Copy link
    Member

    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.

    @terryjreedy terryjreedy added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels May 28, 2011
    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 27, 2013

    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.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jun 17, 2014

    Can someone review the patch please.

    @terryjreedy
    Copy link
    Member

    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.

    @tirkarthi
    Copy link
    Member

    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

    @tirkarthi tirkarthi added 3.7 (EOL) end of life 3.8 only security fixes labels Mar 18, 2019
    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Mar 18, 2019

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

    @vadmium
    Copy link
    Member

    vadmium commented Apr 25, 2019

    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.

    @tirkarthi
    Copy link
    Member

    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)

    @miss-islington
    Copy link
    Contributor

    New changeset bb41147 by Miss Islington (bot) (Xtreak) in branch 'master':
    bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921)
    bb41147

    @miss-islington
    Copy link
    Contributor

    New changeset 44cb89a by Miss Islington (bot) in branch '3.8':
    bpo-12144: Handle cookies with expires attribute in CookieJar.make_cookies (GH-13921)
    44cb89a

    @asvetlov asvetlov added the 3.9 only security fixes label Sep 13, 2019
    @asvetlov
    Copy link
    Contributor

    New changeset e7b7edf 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)
    e7b7edf

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants