Merge pull request #2117 from Pylons/feature/int_conversion_check

Supersedes: #2050 int conversion checks

6 files changed, 116 insertions, 11 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 114bdc3a0..739eb870d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -232,6 +232,11 @@ Bug Fixes
   shell a little more straightfoward.
   See https://github.com/Pylons/pyramid/pull/1883
 
+- Fix an issue when user passes unparsed strings to ``pyramid.session.CookieSession``
+  and ``pyramid.authentication.AuthTktCookieHelper`` for time related parameters
+  ``timeout``, ``reissue_time``, ``max_age`` that expect an integer value.
+  See https://github.com/Pylons/pyramid/pull/2050
+
 - Fixed usage of ``pserve --monitor-restart --daemon`` which would fail in
   horrible ways. See https://github.com/Pylons/pyramid/pull/2118
 
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 2ef07af75..4edf1b4e9 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -254,3 +254,5 @@ Contributors
 - Jesse Dhillon, 2015/10/07
 
 - Amos Latteier, 2015/10/22
+
+- Rami Chousein, 2015/10/28
diff --git a/pyramid/authentication.py b/pyramid/authentication.py
index 0924b5901..9bf1de62e 100644
--- a/pyramid/authentication.py
+++ b/pyramid/authentication.py
@@ -855,9 +855,9 @@ class AuthTktCookieHelper(object):
         self.cookie_name = cookie_name
         self.secure = secure
         self.include_ip = include_ip
-        self.timeout = timeout
-        self.reissue_time = reissue_time
-        self.max_age = max_age
+        self.timeout = timeout if timeout is None else int(timeout)
+        self.reissue_time = reissue_time if reissue_time is None else int(reissue_time)
+        self.max_age = max_age if max_age is None else int(max_age)
         self.wild_domain = wild_domain
         self.parent_domain = parent_domain
         self.domain = domain
@@ -977,8 +977,7 @@ class AuthTktCookieHelper(object):
           Tokens are available in the returned identity when an auth_tkt is
           found in the request and unpacked.  Default: ``()``.
         """
-        if max_age is None:
-            max_age = self.max_age
+        max_age = self.max_age if max_age is None else int(max_age)
 
         environ = request.environ
 
diff --git a/pyramid/session.py b/pyramid/session.py
index c4cfc1949..fa85fe69c 100644
--- a/pyramid/session.py
+++ b/pyramid/session.py
@@ -238,14 +238,14 @@ def BaseCookieSessionFactory(
 
         # configuration parameters
         _cookie_name = cookie_name
-        _cookie_max_age = max_age
+        _cookie_max_age = max_age if max_age is None else int(max_age)
         _cookie_path = path
         _cookie_domain = domain
         _cookie_secure = secure
         _cookie_httponly = httponly
         _cookie_on_exception = set_on_exception
-        _timeout = timeout
-        _reissue_time = reissue_time
+        _timeout = timeout if timeout is None else int(timeout)
+        _reissue_time = reissue_time if reissue_time is None else int(reissue_time)
 
         # dirty flag
         _dirty = False
diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py
index f1308f89e..0a22e5965 100644
--- a/pyramid/tests/test_authentication.py
+++ b/pyramid/tests/test_authentication.py
@@ -600,6 +600,15 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         cookies.load(cookie)
         return cookies.get('auth_tkt')
 
+    def test_init_cookie_str_reissue_invalid(self):
+        self.assertRaises(ValueError, self._makeOne, 'secret', reissue_time='invalid value')
+
+    def test_init_cookie_str_timeout_invalid(self):
+        self.assertRaises(ValueError, self._makeOne, 'secret', timeout='invalid value')
+
+    def test_init_cookie_str_max_age_invalid(self):
+        self.assertRaises(ValueError, self._makeOne, 'secret', max_age='invalid value')
+
     def test_identify_nocookie(self):
         helper = self._makeOne('secret')
         request = self._makeRequest()
@@ -752,11 +761,24 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         result = helper.identify(request)
         self.assertEqual(result, None)
 
-    def test_identify_cookie_timed_out(self):
+    def test_identify_cookie_timeout(self):
         helper = self._makeOne('secret', timeout=1)
-        request = self._makeRequest({'HTTP_COOKIE':'auth_tkt=bogus'})
+        self.assertEqual(helper.timeout, 1)
+
+    def test_identify_cookie_str_timeout(self):
+        helper = self._makeOne('secret', timeout='1')
+        self.assertEqual(helper.timeout, 1)
+
+    def test_identify_cookie_timeout_aged(self):
+        import time
+        helper = self._makeOne('secret', timeout=10)
+        now = time.time()
+        helper.auth_tkt.timestamp = now - 1
+        helper.now = now + 10
+        helper.auth_tkt.tokens = (text_('a'), )
+        request = self._makeRequest('bogus')
         result = helper.identify(request)
-        self.assertEqual(result, None)
+        self.assertFalse(result)
 
     def test_identify_cookie_reissue(self):
         import time
@@ -774,6 +796,22 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         self.assertEqual(len(response.headerlist), 3)
         self.assertEqual(response.headerlist[0][0], 'Set-Cookie')
 
+    def test_identify_cookie_str_reissue(self):
+        import time
+        helper = self._makeOne('secret', timeout=10, reissue_time='0')
+        now = time.time()
+        helper.auth_tkt.timestamp = now
+        helper.now = now + 1
+        helper.auth_tkt.tokens = (text_('a'), )
+        request = self._makeRequest('bogus')
+        result = helper.identify(request)
+        self.assertTrue(result)
+        self.assertEqual(len(request.callbacks), 1)
+        response = DummyResponse()
+        request.callbacks[0](request, response)
+        self.assertEqual(len(response.headerlist), 3)
+        self.assertEqual(response.headerlist[0][0], 'Set-Cookie')
+
     def test_identify_cookie_reissue_already_reissued_this_request(self):
         import time
         helper = self._makeOne('secret', timeout=10, reissue_time=0)
@@ -1060,6 +1098,16 @@ class TestAuthTktCookieHelper(unittest.TestCase):
     def test_remember_max_age(self):
         helper = self._makeOne('secret')
         request = self._makeRequest()
+        result = helper.remember(request, 'userid', max_age=500)
+        values = self._parseHeaders(result)
+        self.assertEqual(len(result), 3)
+
+        self.assertEqual(values[0]['max-age'], '500')
+        self.assertTrue(values[0]['expires'])
+
+    def test_remember_str_max_age(self):
+        helper = self._makeOne('secret')
+        request = self._makeRequest()
         result = helper.remember(request, 'userid', max_age='500')
         values = self._parseHeaders(result)
         self.assertEqual(len(result), 3)
@@ -1067,6 +1115,11 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         self.assertEqual(values[0]['max-age'], '500')
         self.assertTrue(values[0]['expires'])
 
+    def test_remember_str_max_age_invalid(self):
+        helper = self._makeOne('secret')
+        request = self._makeRequest()
+        self.assertRaises(ValueError, helper.remember, request, 'userid', max_age='invalid value')
+
     def test_remember_tokens(self):
         helper = self._makeOne('secret')
         request = self._makeRequest()
diff --git a/pyramid/tests/test_session.py b/pyramid/tests/test_session.py
index eac6593d9..82e4fb001 100644
--- a/pyramid/tests/test_session.py
+++ b/pyramid/tests/test_session.py
@@ -62,6 +62,18 @@ class SharedCookieSessionTests(object):
         session = self._makeOne(request, timeout=None)
         self.assertEqual(dict(session), {'state': 1})
 
+    def test_timeout_str(self):
+        import time
+        request = testing.DummyRequest()
+        cookieval = self._serialize((time.time() - 5, 0, {'state': 1}))
+        request.cookies['session'] = cookieval
+        session = self._makeOne(request, timeout='1')
+        self.assertEqual(dict(session), {})
+
+    def test_timeout_invalid(self):
+        request = testing.DummyRequest()
+        self.assertRaises(ValueError, self._makeOne, request, timeout='Invalid value')
+
     def test_changed(self):
         request = testing.DummyRequest()
         session = self._makeOne(request)
@@ -297,6 +309,23 @@ class TestBaseCookieSession(SharedCookieSessionTests, unittest.TestCase):
         self.assertEqual(session['state'], 1)
         self.assertFalse(session._dirty)
 
+    def test_reissue_str_triggered(self):
+        import time
+        request = testing.DummyRequest()
+        cookieval = self._serialize((time.time() - 2, 0, {'state': 1}))
+        request.cookies['session'] = cookieval
+        session = self._makeOne(request, reissue_time='0')
+        self.assertEqual(session['state'], 1)
+        self.assertTrue(session._dirty)
+
+    def test_reissue_invalid(self):
+        request = testing.DummyRequest()
+        self.assertRaises(ValueError, self._makeOne, request, reissue_time='invalid value')
+
+    def test_cookie_max_age_invalid(self):
+        request = testing.DummyRequest()
+        self.assertRaises(ValueError, self._makeOne, request, max_age='invalid value')
+
 class TestSignedCookieSession(SharedCookieSessionTests, unittest.TestCase):
     def _makeOne(self, request, **kw):
         from pyramid.session import SignedCookieSessionFactory
@@ -331,6 +360,23 @@ class TestSignedCookieSession(SharedCookieSessionTests, unittest.TestCase):
         self.assertEqual(session['state'], 1)
         self.assertFalse(session._dirty)
 
+    def test_reissue_str_triggered(self):
+        import time
+        request = testing.DummyRequest()
+        cookieval = self._serialize((time.time() - 2, 0, {'state': 1}))
+        request.cookies['session'] = cookieval
+        session = self._makeOne(request, reissue_time='0')
+        self.assertEqual(session['state'], 1)
+        self.assertTrue(session._dirty)
+
+    def test_reissue_invalid(self):
+        request = testing.DummyRequest()
+        self.assertRaises(ValueError, self._makeOne, request, reissue_time='invalid value')
+
+    def test_cookie_max_age_invalid(self):
+        request = testing.DummyRequest()
+        self.assertRaises(ValueError, self._makeOne, request, max_age='invalid value')
+
     def test_custom_salt(self):
         import time
         request = testing.DummyRequest()