From f5012b423f6586663dc79c0374c6d986a2a5da78 Mon Sep 17 00:00:00 2001
From: Ben Darnell <ben@bendarnell.com>
Date: Sun, 6 May 2012 17:29:02 -0700
Subject: [PATCH] Document and test for type differences between get_cookie and
 get_secure_cookie.

Closes #501.
---
 tornado/test/web_test.py | 25 +++++++++++++++++++++----
 tornado/web.py           |  9 ++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 49d67de5..40aafbc1 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -5,7 +5,7 @@ from tornado.iostream import IOStream
 from tornado.template import DictLoader
 from tornado.testing import LogTrapTestCase, AsyncHTTPTestCase
 from tornado.util import b, bytes_type, ObjectDict
-from tornado.web import RequestHandler, authenticated, Application, asynchronous, url, HTTPError, StaticFileHandler, _create_signature
+from tornado.web import RequestHandler, authenticated, Application, asynchronous, url, HTTPError, StaticFileHandler, _create_signature, create_signed_value
 
 import binascii
 import logging
@@ -60,6 +60,13 @@ class SecureCookieTest(LogTrapTestCase):
         # it gets rejected
         assert handler.get_secure_cookie('foo') is None
 
+    def test_arbitrary_bytes(self):
+        # Secure cookies accept arbitrary data (which is base64 encoded).
+        # Note that normal cookies accept only a subset of ascii.
+        handler = CookieTestRequestHandler()
+        handler.set_secure_cookie('foo', b('\xe9'))
+        self.assertEqual(handler.get_secure_cookie('foo'), b('\xe9'))
+
 
 class CookieTest(AsyncHTTPTestCase, LogTrapTestCase):
     def get_app(self):
@@ -294,6 +301,12 @@ class TypeCheckHandler(RequestHandler):
         self.check_type('cookie_key', self.cookies.keys()[0], str)
         self.check_type('cookie_value', self.cookies.values()[0].value, str)
 
+        # Secure cookies return bytes because they can contain arbitrary
+        # data, but regular cookies are native strings.
+        assert self.cookies.keys() == ['asdf']
+        self.check_type('get_secure_cookie', self.get_secure_cookie('asdf'), bytes_type)
+        self.check_type('get_cookie', self.get_cookie('asdf'), str)
+
         self.check_type('xsrf_token', self.xsrf_token, bytes_type)
         self.check_type('xsrf_form_html', self.xsrf_form_html(), str)
 
@@ -413,6 +426,7 @@ class HeaderInjectionHandler(RequestHandler):
 
 
 class WebTest(AsyncHTTPTestCase, LogTrapTestCase):
+    COOKIE_SECRET = "WebTest.COOKIE_SECRET"
     def get_app(self):
         loader = DictLoader({
                 "linkify.html": "{% module linkify(message) %}",
@@ -441,7 +455,8 @@ class WebTest(AsyncHTTPTestCase, LogTrapTestCase):
             ]
         return Application(urls,
                            template_loader=loader,
-                           autoescape="xhtml_escape")
+                           autoescape="xhtml_escape",
+                           cookie_secret=self.COOKIE_SECRET)
 
     def fetch_json(self, *args, **kwargs):
         response = self.fetch(*args, **kwargs)
@@ -449,13 +464,15 @@ class WebTest(AsyncHTTPTestCase, LogTrapTestCase):
         return json_decode(response.body)
 
     def test_types(self):
+        cookie_value = to_unicode(create_signed_value(self.COOKIE_SECRET,
+                                                      "asdf", "qwer"))
         response = self.fetch("/typecheck/asdf?foo=bar",
-                              headers={"Cookie": "cook=ie"})
+                              headers={"Cookie": "asdf=" + cookie_value})
         data = json_decode(response.body)
         self.assertEqual(data, {})
 
         response = self.fetch("/typecheck/asdf?foo=bar", method="POST",
-                              headers={"Cookie": "cook=ie"},
+                              headers={"Cookie": "asdf=" + cookie_value},
                               body="foo=bar")
 
     def test_decode_argument(self):
diff --git a/tornado/web.py b/tornado/web.py
index cb8a68bb..8978d2d0 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -408,6 +408,9 @@ class RequestHandler(object):
         Note that the ``expires_days`` parameter sets the lifetime of the
         cookie in the browser, but is independent of the ``max_age_days``
         parameter to `get_secure_cookie`.
+
+        Secure cookies may contain arbitrary byte values, not just unicode
+        strings (unlike regular cookies)
         """
         self.set_cookie(name, self.create_signed_value(name, value),
                         expires_days=expires_days, **kwargs)
@@ -424,7 +427,11 @@ class RequestHandler(object):
                                    name, value)
 
     def get_secure_cookie(self, name, value=None, max_age_days=31):
-        """Returns the given signed cookie if it validates, or None."""
+        """Returns the given signed cookie if it validates, or None.
+
+        The decoded cookie value is returned as a byte string (unlike
+        `get_cookie`).
+        """
         self.require_setting("cookie_secret", "secure cookies")
         if value is None:
             value = self.get_cookie(name)