Use a single Cookie.SimpleCookie for outgoing cookies instead of a list.

The main reason for this change is to repeated calls to set_cookie
overwrite rather than append.

Closes #445.

tornado/test/web_test.py

@@ -87,19 +87,29 @@ class CookieTest(AsyncHTTPTestCase, LogTrapTestCase):
                 self.set_cookie("semicolon", "a;b")
                 self.set_cookie("quote", 'a"b')
 
+        class SetCookieOverwriteHandler(RequestHandler):
+            def get(self):
+                self.set_cookie("a", "b", domain="example.com")
+                self.set_cookie("c", "d" ,domain="example.com")
+                # A second call with the same name clobbers the first.
+                # Attributes from the first call are not carried over.
+                self.set_cookie("a", "e")
+
         return Application([
                 ("/set", SetCookieHandler),
                 ("/get", GetCookieHandler),
                 ("/set_domain", SetCookieDomainHandler),
                 ("/special_char", SetCookieSpecialCharHandler),
+                ("/set_overwrite", SetCookieOverwriteHandler),
                 ])
 
     def test_set_cookie(self):
         response = self.fetch("/set")
-        self.assertEqual(response.headers.get_list("Set-Cookie"),
-                         ["str=asdf; Path=/",
+        self.assertEqual(sorted(response.headers.get_list("Set-Cookie")),
+                         ["bytes=zxcv; Path=/",
+                          "str=asdf; Path=/",
                           "unicode=qwer; Path=/",
-                          "bytes=zxcv; Path=/"])
+                          ])
 
     def test_get_cookie(self):
         response = self.fetch("/get", headers={"Cookie": "foo=bar"})
@@ -118,14 +128,14 @@ class CookieTest(AsyncHTTPTestCase, LogTrapTestCase):
 
     def test_cookie_special_char(self):
         response = self.fetch("/special_char")
-        headers = response.headers.get_list("Set-Cookie")
+        headers = sorted(response.headers.get_list("Set-Cookie"))
         self.assertEqual(len(headers), 3)
         self.assertEqual(headers[0], 'equals="a=b"; Path=/')
+        self.assertEqual(headers[1], 'quote="a\\"b"; Path=/')
         # python 2.7 octal-escapes the semicolon; older versions leave it alone
-        self.assertTrue(headers[1] in ('semicolon="a;b"; Path=/',
+        self.assertTrue(headers[2] in ('semicolon="a;b"; Path=/',
                                        'semicolon="a\\073b"; Path=/'),
-                        headers[1])
-        self.assertEqual(headers[2], 'quote="a\\"b"; Path=/')
+                        headers[2])
 
         data = [('foo=a=b', 'a=b'),
                 ('foo="a=b"', 'a=b'),
@@ -139,6 +149,12 @@ class CookieTest(AsyncHTTPTestCase, LogTrapTestCase):
             response = self.fetch("/get", headers={"Cookie": header})
             self.assertEqual(response.body, utf8(expected))
 
+    def test_set_cookie_overwrite(self):
+        response = self.fetch("/set_overwrite")
+        headers = response.headers.get_list("Set-Cookie")
+        self.assertEqual(sorted(headers),
+                         ["a=e; Path=/", "c=d; Domain=example.com; Path=/"])
+
 
 class AuthRedirectRequestHandler(RequestHandler):
     def initialize(self, login_url):

tornado/web.py

@@ -359,26 +359,27 @@ class RequestHandler(object):
         if re.search(r"[\x00-\x20]", name + value):
             # Don't let us accidentally inject bad stuff
             raise ValueError("Invalid cookie %r: %r" % (name, value))
-        if not hasattr(self, "_new_cookies"):
-            self._new_cookies = []
-        new_cookie = Cookie.SimpleCookie()
-        self._new_cookies.append(new_cookie)
-        new_cookie[name] = value
+        if not hasattr(self, "_new_cookie"):
+            self._new_cookie = Cookie.SimpleCookie()
+        if name in self._new_cookie:
+            del self._new_cookie[name]
+        self._new_cookie[name] = value
+        morsel = self._new_cookie[name]
         if domain:
-            new_cookie[name]["domain"] = domain
+            morsel["domain"] = domain
         if expires_days is not None and not expires:
             expires = datetime.datetime.utcnow() + datetime.timedelta(
                 days=expires_days)
         if expires:
             timestamp = calendar.timegm(expires.utctimetuple())
-            new_cookie[name]["expires"] = email.utils.formatdate(
+            morsel["expires"] = email.utils.formatdate(
                 timestamp, localtime=False, usegmt=True)
         if path:
-            new_cookie[name]["path"] = path
+            morsel["path"] = path
         for k, v in kwargs.iteritems():
             if k == 'max_age':
                 k = 'max-age'
-            new_cookie[name][k] = v
+            morsel[k] = v
 
     def clear_cookie(self, name, path="/", domain=None):
         """Deletes the cookie with the given name."""
@@ -1007,8 +1008,8 @@ class RequestHandler(object):
                       " " + httplib.responses[self._status_code])]
         lines.extend([(utf8(n) + b(": ") + utf8(v)) for n, v in
                       itertools.chain(self._headers.iteritems(), self._list_headers)])
-        for cookie_dict in getattr(self, "_new_cookies", []):
-            for cookie in cookie_dict.values():
+        if hasattr(self, "_new_cookie"):
+            for cookie in self._new_cookie.values():
                 lines.append(utf8("Set-Cookie: " + cookie.OutputString(None)))
         return b("\r\n").join(lines) + b("\r\n\r\n")
 

website/sphinx/releases/next.rst

@@ -9,3 +9,5 @@ In progress
   the upcoming release of Python 3.3.
 * `tornado.simple_httpclient` is better about closing its sockets
   instead of leaving them for garbage collection.
+* Repeated calls to `RequestHandler.set_cookie` with the same name now
+  overwrite the previous cookie instead of producing additional copies.