Key versioning: review comments addressed

diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 56701a992c0b65affb9ff93be88b339767cb3374..f5bdc2e3556fb1d72852d8cab0055b923e2d018f 100644 (file)
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -138,6 +138,7 @@ class SecureCookieV2Test(unittest.TestCase):
         0: 'ajklasdf0ojaisdf',
         1: 'aslkjasaolwkjsdf'
     }
+
     def test_round_trip(self):
         handler = CookieTestRequestHandler()
         handler.set_secure_cookie('foo', b'bar', version=2)
@@ -149,6 +150,12 @@ class SecureCookieV2Test(unittest.TestCase):
         handler.set_secure_cookie('foo', b'bar')
         self.assertEqual(handler.get_secure_cookie('foo'), b'bar')
 
+    def test_key_version_roundtrip_differing_version(self):
+        handler = CookieTestRequestHandler(cookie_secret=self.KEY_VERSIONS,
+                                           key_version=1)
+        handler.set_secure_cookie('foo', b'bar')
+        self.assertEqual(handler.get_secure_cookie('foo'), b'bar')
+
     def test_key_version_increment_version(self):
         handler = CookieTestRequestHandler(cookie_secret=self.KEY_VERSIONS,
                                            key_version=0)
@@ -160,10 +167,10 @@ class SecureCookieV2Test(unittest.TestCase):
 
     def test_key_version_invalidate_version(self):
         handler = CookieTestRequestHandler(cookie_secret=self.KEY_VERSIONS,
-                                           key_version=1)
+                                           key_version=0)
         handler.set_secure_cookie('foo', b'bar')
         new_key_versions = self.KEY_VERSIONS.copy()
-        new_key_versions.pop(1)
+        new_key_versions.pop(0)
         new_handler = CookieTestRequestHandler(cookie_secret=new_key_versions,
                                                key_version=1)
         new_handler._cookies = handler._cookies
@@ -2291,7 +2298,8 @@ class SignedValueTest(unittest.TestCase):
     def test_key_versioning_read_write_default_key(self):
         value = b"\xe9"
         signed = create_signed_value(SignedValueTest.SECRET_DICT,
-                                     "key", value, clock=self.present)
+                                     "key", value, clock=self.present,
+                                     key_version=0)
         decoded = decode_signed_value(SignedValueTest.SECRET_DICT,
                                       "key", signed, clock=self.present)
         self.assertEqual(value, decoded)
@@ -2308,14 +2316,15 @@ class SignedValueTest(unittest.TestCase):
     def test_key_versioning_invalid_key(self):
         value = b"\xe9"
         signed = create_signed_value(SignedValueTest.SECRET_DICT,
-                                     "key", value, clock=self.present)
+                                     "key", value, clock=self.present,
+                                     key_version=0)
         newkeys = SignedValueTest.SECRET_DICT.copy()
         newkeys.pop(0)
         decoded = decode_signed_value(newkeys,
                                       "key", signed, clock=self.present)
         self.assertEqual(None, decoded)
 
-    def test_key_version_retreival(self):
+    def test_key_version_retrieval(self):
         value = b"\xe9"
         signed = create_signed_value(SignedValueTest.SECRET_DICT,
                                      "key", value, clock=self.present,

diff --git a/tornado/web.py b/tornado/web.py
index 457fc6faf9b4fac9a14227183671cf7d1cf7f849..d463a77fca6a338b3c1e72d5a7e66fbef087e1c9 100644 (file)
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -144,15 +144,6 @@ May be overridden by passing a ``min_version`` keyword argument.
 .. versionadded:: 3.2.1
 """
 
-DEFAULT_SIGN_KEY_VERSION = 0
-"""The current key index used by `.RequestHandler.set_secure_cookie`.
-
-May be overridden by passing a ``key_version`` keyword argument.
-
-.. versionadded:: x.x.x
-"""
-
-
 class RequestHandler(object):
     """Subclass this class and define `get()` or `post()` to make a handler.
 
@@ -2994,11 +2985,6 @@ def create_signed_value(secret, name, value, version=None, clock=None,
     if clock is None:
         clock = time.time
 
-    if key_version is None:
-        key_version = DEFAULT_SIGN_KEY_VERSION
-    else:
-        assert version >= 2, 'Version must be at least 2 for key version support'
-
     timestamp = utf8(str(int(clock())))
     value = base64.b64encode(utf8(value))
     if version == 1:
@@ -3024,13 +3010,15 @@ def create_signed_value(secret, name, value, version=None, clock=None,
             return utf8("%d:" % len(s)) + utf8(s)
         to_sign = b"|".join([
             b"2",
-            format_field(str(key_version)),
+            format_field(str(key_version or 0)),
             format_field(timestamp),
             format_field(name),
             format_field(value),
             b''])
 
         if isinstance(secret, dict):
+            assert key_version is not None, 'Key version must be set when sign key dict is used'
+            assert version >= 2, 'Version must be at least 2 for key version support'
             secret = secret[key_version]
 
         signature = _create_signature_v2(secret, to_sign)