From 70ea7a5a6ca956b613168c9d123ee7cc951054bf Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Sun, 19 Feb 2012 18:47:46 -0800 Subject: [PATCH] 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 | 30 +++++++++++++++++++++++------- tornado/web.py | 23 ++++++++++++----------- website/sphinx/releases/next.rst | 2 ++ 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 32d4188a..c292b419 100644 --- a/tornado/test/web_test.py +++ b/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): diff --git a/tornado/web.py b/tornado/web.py index a616eb2c..8bdf0ddb 100644 --- a/tornado/web.py +++ b/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") diff --git a/website/sphinx/releases/next.rst b/website/sphinx/releases/next.rst index b1149b5a..107507ce 100644 --- a/website/sphinx/releases/next.rst +++ b/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.