Permalink
Browse files

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.
  • Loading branch information...
1 parent 2539f83 commit 70ea7a5a6ca956b613168c9d123ee7cc951054bf @bdarnell bdarnell committed Feb 20, 2012
Showing with 37 additions and 18 deletions.
  1. +23 −7 tornado/test/web_test.py
  2. +12 −11 tornado/web.py
  3. +2 −0 website/sphinx/releases/next.rst
View
@@ -87,19 +87,29 @@ def get(self):
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 @@ def test_set_cookie_domain(self):
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 @@ def test_cookie_special_char(self):
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):
View
@@ -359,26 +359,27 @@ def set_cookie(self, name, value, domain=None, expires=None, path="/",
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 @@ def _generate_headers(self):
" " + 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")
@@ -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.

0 comments on commit 70ea7a5

Please sign in to comment.