From 39841562c5a5d34ed6bebbf733f97b95d57977ca Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Mon, 2 Dec 2013 22:14:41 -0500 Subject: [PATCH 1/7] test_requests: Add tests for morsel_to_cookie These tests will ensure my changes to how we handle 'expires' don't cause any regressions. --- test_requests.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/test_requests.py b/test_requests.py index a800ba71..41341534 100755 --- a/test_requests.py +++ b/test_requests.py @@ -6,15 +6,16 @@ from __future__ import division import json import os -import unittest import pickle +import unittest import requests import pytest -from requests.auth import HTTPDigestAuth from requests.adapters import HTTPAdapter -from requests.compat import str, cookielib, getproxies, urljoin, urlparse -from requests.cookies import cookiejar_from_dict +from requests.auth import HTTPDigestAuth +from requests.compat import ( + Morsel, cookielib, getproxies, str, urljoin, urlparse) +from requests.cookies import cookiejar_from_dict, morsel_to_cookie from requests.exceptions import InvalidURL, MissingSchema from requests.structures import CaseInsensitiveDict @@ -759,6 +760,24 @@ class RequestsTestCase(unittest.TestCase): preq = req.prepare() assert test_url == preq.url + def test_morsel_to_cookie_expires_is_converted(self): + morsel = Morsel() + + # Test case where we convert from string time + morsel['expires'] = 'Thu, 01-Jan-1970 00:00:01 GMT' + cookie = morsel_to_cookie(morsel) + self.assertEquals(cookie.expires, 18001) + + # Test case where no conversion is required + morsel['expires'] = 100 + cookie = morsel_to_cookie(morsel) + self.assertEquals(cookie.expires, 100) + + # Test case where an invalid string is input + morsel['expires'] = 'woops' + with self.assertRaises(ValueError): + cookie = morsel_to_cookie(morsel) + class TestContentEncodingDetection(unittest.TestCase): From 6140d082416eb70730940181fe19b723d72e0311 Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Mon, 2 Dec 2013 22:15:01 -0500 Subject: [PATCH 2/7] cookies: Replace type checks with try/except --- requests/cookies.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/requests/cookies.py b/requests/cookies.py index c465f552..29b60f84 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -383,9 +383,11 @@ def morsel_to_cookie(morsel): expires = time.time() + morsel["max-age"] elif morsel['expires']: expires = morsel['expires'] - if type(expires) == type(""): + try: time_template = "%a, %d-%b-%Y %H:%M:%S GMT" expires = time.mktime(time.strptime(expires, time_template)) + except TypeError: + pass c = create_cookie( name=morsel.key, value=morsel.value, From 6ac70450dc4c5099ec01cb7d9bf1a1ab653370b4 Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Mon, 2 Dec 2013 22:15:16 -0500 Subject: [PATCH 3/7] AUTHORS: Take credit --- AUTHORS.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index b0a7e241..e94a4bff 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -144,3 +144,4 @@ Patches and Suggestions - Jayson Vantuyl @kagato - Pengfei.X - Kamil Madac +- Michael Becker @beckerfuffle From ef5875cc19d8ca34a37662f53e196944595874d8 Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Tue, 3 Dec 2013 11:00:25 -0500 Subject: [PATCH 4/7] cookies: Take a less cowardly approach Per discussion with @sigmavirus24, we'll take the less cowardly approach here. "the RFC defines the Expires header as a string with the format we're assigning to ``time_template`` so checking if it is a string is unnecessary." Crossing my fingers that this doesn't break anyones existing code... --- requests/cookies.py | 37 +++++++++++------------ test_requests.py | 74 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/requests/cookies.py b/requests/cookies.py index 29b60f84..af33b2e8 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -378,31 +378,28 @@ def create_cookie(name, value, **kwargs): def morsel_to_cookie(morsel): """Convert a Morsel object into a Cookie containing the one k/v pair.""" + expires = None - if morsel["max-age"]: - expires = time.time() + morsel["max-age"] + if morsel['max-age']: + expires = time.time() + morsel['max-age'] elif morsel['expires']: - expires = morsel['expires'] - try: - time_template = "%a, %d-%b-%Y %H:%M:%S GMT" - expires = time.mktime(time.strptime(expires, time_template)) - except TypeError: - pass - c = create_cookie( - name=morsel.key, - value=morsel.value, - version=morsel['version'] or 0, - port=None, - domain=morsel['domain'], - path=morsel['path'], - secure=bool(morsel['secure']), - expires=expires, - discard=False, + time_template = '%a, %d-%b-%Y %H:%M:%S GMT' + expires = time.mktime(time.strptime(morsel['expires'], time_template)) + return create_cookie( comment=morsel['comment'], comment_url=bool(morsel['comment']), + discard=False, + domain=morsel['domain'], + expires=expires, + name=morsel.key, + path=morsel['path'], + port=None, rest={'HttpOnly': morsel['httponly']}, - rfc2109=False,) - return c + rfc2109=False, + secure=bool(morsel['secure']), + value=morsel.value, + version=morsel['version'] or 0, + ) def cookiejar_from_dict(cookie_dict, cookiejar=None, overwrite=True): diff --git a/test_requests.py b/test_requests.py index 41341534..d999a483 100755 --- a/test_requests.py +++ b/test_requests.py @@ -760,24 +760,6 @@ class RequestsTestCase(unittest.TestCase): preq = req.prepare() assert test_url == preq.url - def test_morsel_to_cookie_expires_is_converted(self): - morsel = Morsel() - - # Test case where we convert from string time - morsel['expires'] = 'Thu, 01-Jan-1970 00:00:01 GMT' - cookie = morsel_to_cookie(morsel) - self.assertEquals(cookie.expires, 18001) - - # Test case where no conversion is required - morsel['expires'] = 100 - cookie = morsel_to_cookie(morsel) - self.assertEquals(cookie.expires, 100) - - # Test case where an invalid string is input - morsel['expires'] = 'woops' - with self.assertRaises(ValueError): - cookie = morsel_to_cookie(morsel) - class TestContentEncodingDetection(unittest.TestCase): @@ -1023,5 +1005,61 @@ class UtilsTestCase(unittest.TestCase): assert not address_in_network('172.16.0.1', '192.168.1.0/24') + +class TestMorselToCookieExpires(unittest.TestCase): + + """Tests for morsel_to_cookie when morsel contains expires.""" + + def test_expires_valid_str(self): + """Test case where we convert expires from string time.""" + + morsel = Morsel() + morsel['expires'] = 'Thu, 01-Jan-1970 00:00:01 GMT' + cookie = morsel_to_cookie(morsel) + self.assertEquals(cookie.expires, 18001) + + def test_expires_invalid_int(self): + """Test case where an invalid type is passed for expires.""" + + morsel = Morsel() + morsel['expires'] = 100 + self.assertRaises(TypeError, morsel_to_cookie, (morsel)) + + def test_expires_invalid_str(self): + """Test case where an invalid string is input.""" + + morsel = Morsel() + morsel['expires'] = 'woops' + self.assertRaises(ValueError, morsel_to_cookie, (morsel)) + + def test_expires_none(self): + """Test case where expires is None.""" + + morsel = Morsel() + morsel['expires'] = None + cookie = morsel_to_cookie(morsel) + self.assertEquals(cookie.expires, None) + + +class TestMorselToCookieMaxAge(unittest.TestCase): + + """Tests for morsel_to_cookie when morsel contains max-age.""" + + def test_max_age_valid_int(self): + """Test case where a valid max age in seconds is passed.""" + + morsel = Morsel() + morsel['max-age'] = 60 + cookie = morsel_to_cookie(morsel) + self.assertIsInstance(cookie.expires, int) + + def test_max_age_invalid_str(self): + """Test case where a invalid max age is passed.""" + + morsel = Morsel() + morsel['max-age'] = 'woops' + self.assertRaises(TypeError, morsel_to_cookie, (morsel)) + + if __name__ == '__main__': unittest.main() From ea4570da5714deac86d6438ebac3f0b2ecea1ee6 Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Thu, 5 Dec 2013 21:54:29 -0500 Subject: [PATCH 5/7] cookies: Fix bugs found during CI --- requests/cookies.py | 3 ++- test_requests.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/requests/cookies.py b/requests/cookies.py index af33b2e8..35dacfc4 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -384,7 +384,8 @@ def morsel_to_cookie(morsel): expires = time.time() + morsel['max-age'] elif morsel['expires']: time_template = '%a, %d-%b-%Y %H:%M:%S GMT' - expires = time.mktime(time.strptime(morsel['expires'], time_template)) + expires = time.mktime( + time.strptime(morsel['expires'], time_template)) - time.timezone return create_cookie( comment=morsel['comment'], comment_url=bool(morsel['comment']), diff --git a/test_requests.py b/test_requests.py index d999a483..de61fd55 100755 --- a/test_requests.py +++ b/test_requests.py @@ -1016,7 +1016,7 @@ class TestMorselToCookieExpires(unittest.TestCase): morsel = Morsel() morsel['expires'] = 'Thu, 01-Jan-1970 00:00:01 GMT' cookie = morsel_to_cookie(morsel) - self.assertEquals(cookie.expires, 18001) + self.assertEquals(cookie.expires, 1) def test_expires_invalid_int(self): """Test case where an invalid type is passed for expires.""" @@ -1051,7 +1051,7 @@ class TestMorselToCookieMaxAge(unittest.TestCase): morsel = Morsel() morsel['max-age'] = 60 cookie = morsel_to_cookie(morsel) - self.assertIsInstance(cookie.expires, int) + self.assertTrue(isinstance(cookie.expires, int)) def test_max_age_invalid_str(self): """Test case where a invalid max age is passed.""" From 837ba94dde2ba0c69ad8bd7a7db02e69ff75a05c Mon Sep 17 00:00:00 2001 From: Michael Becker Date: Fri, 6 Dec 2013 10:08:42 -0500 Subject: [PATCH 6/7] test_requests: convert tests to py.test style --- test_requests.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test_requests.py b/test_requests.py index de61fd55..093b37c1 100755 --- a/test_requests.py +++ b/test_requests.py @@ -1016,21 +1016,23 @@ class TestMorselToCookieExpires(unittest.TestCase): morsel = Morsel() morsel['expires'] = 'Thu, 01-Jan-1970 00:00:01 GMT' cookie = morsel_to_cookie(morsel) - self.assertEquals(cookie.expires, 1) + assert cookie.expires == 1 def test_expires_invalid_int(self): """Test case where an invalid type is passed for expires.""" morsel = Morsel() morsel['expires'] = 100 - self.assertRaises(TypeError, morsel_to_cookie, (morsel)) + with pytest.raises(TypeError): + morsel_to_cookie(morsel) def test_expires_invalid_str(self): """Test case where an invalid string is input.""" morsel = Morsel() morsel['expires'] = 'woops' - self.assertRaises(ValueError, morsel_to_cookie, (morsel)) + with pytest.raises(ValueError): + morsel_to_cookie(morsel) def test_expires_none(self): """Test case where expires is None.""" @@ -1038,7 +1040,7 @@ class TestMorselToCookieExpires(unittest.TestCase): morsel = Morsel() morsel['expires'] = None cookie = morsel_to_cookie(morsel) - self.assertEquals(cookie.expires, None) + assert cookie.expires is None class TestMorselToCookieMaxAge(unittest.TestCase): @@ -1051,14 +1053,15 @@ class TestMorselToCookieMaxAge(unittest.TestCase): morsel = Morsel() morsel['max-age'] = 60 cookie = morsel_to_cookie(morsel) - self.assertTrue(isinstance(cookie.expires, int)) + assert isinstance(cookie.expires, int) def test_max_age_invalid_str(self): """Test case where a invalid max age is passed.""" morsel = Morsel() morsel['max-age'] = 'woops' - self.assertRaises(TypeError, morsel_to_cookie, (morsel)) + with pytest.raises(TypeError): + morsel_to_cookie(morsel) if __name__ == '__main__': From 456c42b00d15c1ffb7fe610e4dad9164ab3eb6f0 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 12 Dec 2013 16:21:04 +0000 Subject: [PATCH 7/7] Fixup changelog with missing breaking change. --- HISTORY.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/HISTORY.rst b/HISTORY.rst index 601ba580..f27ce332 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -7,6 +7,7 @@ Release History ++++++++++++++++++ - Updated CA Bundle, of course. +- Cookies set on individual Requests through a ``Session`` (e.g. via ``Session.get()``) are no longer persisted to the ``Session``. - Clean up connections when we hit problems during chunked upload, rather than leaking them. - Return connections to the pool when a chunked upload is successful, rather than leaking it. - Match the HTTPbis recommendation for HTTP 301 redirects.