From b1df6635876022252d73fc1c70a0e8682b4bedf9 Mon Sep 17 00:00:00 2001
From: Ben Darnell <ben@bendarnell.com>
Date: Sun, 20 May 2012 16:51:02 -0700
Subject: [PATCH] 304 responses no longer include entity headers like
 Content-Length

This is required by the RFC as it may confuse caches.
---
 tornado/test/web_test.py | 51 ++++++++++++++++++++++++++++++++++++++++
 tornado/web.py           | 29 +++++++++++++++++++++--
 2 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index ff2f9f80..8d3675b9 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -15,6 +15,16 @@ import socket
 import sys
 
 
+class SimpleHandlerTestCase(AsyncHTTPTestCase):
+    """Simplified base class for tests that work with a single handler class.
+
+    To use, define a nested class named ``Handler``.
+    """
+    def get_app(self):
+        return Application([('/', self.Handler)],
+                           log_function=lambda x: None)
+
+
 class CookieTestRequestHandler(RequestHandler):
     # stub out enough methods to make the secure_cookie functions work
     def __init__(self):
@@ -714,6 +724,14 @@ class StaticFileTest(AsyncHTTPTestCase, LogTrapTestCase):
         response = self.fetch(path % int(include_host))
         self.assertEqual(response.body, utf8(str(True)))
 
+    def test_static_304(self):
+        response1 = self.fetch("/static/robots.txt")
+        response2 = self.fetch("/static/robots.txt", headers={
+                'If-Modified-Since': response1.headers['Last-Modified']})
+        self.assertEqual(response2.code, 304)
+        self.assertTrue('Content-Length' not in response2.headers)
+        self.assertTrue('Last-Modified' not in response2.headers)
+
 
 class CustomStaticFileTest(AsyncHTTPTestCase, LogTrapTestCase):
     def get_app(self):
@@ -769,3 +787,36 @@ class NamedURLSpecGroupsTest(AsyncHTTPTestCase, LogTrapTestCase):
 
         response = self.fetch("/unicode/bar")
         self.assertEqual(response.body, b("bar"))
+
+class ClearHeaderTest(SimpleHandlerTestCase):
+    class Handler(RequestHandler):
+        def get(self):
+            self.set_header("h1", "foo")
+            self.set_header("h2", "bar")
+            self.clear_header("h1")
+            self.clear_header("nonexistent")
+
+    def test_clear_header(self):
+        response = self.fetch("/")
+        self.assertTrue("h1" not in response.headers)
+        self.assertEqual(response.headers["h2"], "bar")
+        
+
+class Header304Test(SimpleHandlerTestCase):
+    class Handler(RequestHandler):
+        def get(self):
+            self.set_header("Content-Language", "en_US")
+            self.write("hello")
+            
+    def test_304_headers(self):
+        response1 = self.fetch('/')
+        self.assertEqual(response1.headers["Content-Length"], "5")
+        self.assertEqual(response1.headers["Content-Language"], "en_US")
+        
+        response2 = self.fetch('/', headers={
+                'If-None-Match': response1.headers["Etag"]})
+        self.assertEqual(response2.code, 304)
+        self.assertTrue("Content-Length" not in response2.headers)
+        self.assertTrue("Content-Language" not in response2.headers)
+        # Not an entity header, but should not be added to 304s by chunking
+        self.assertTrue("Transfer-Encoding" not in response2.headers)
diff --git a/tornado/web.py b/tornado/web.py
index b5fe74d4..b90324f9 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -264,6 +264,15 @@ class RequestHandler(object):
         """
         self._list_headers.append((name, self._convert_header_value(value)))
 
+    def clear_header(self, name):
+        """Clears an outgoing header, undoing a previous `set_header` call.
+
+        Note that this method does not apply to multi-valued headers
+        set by `add_header`.
+        """
+        if name in self._headers:
+            del self._headers[name]
+
     def _convert_header_value(self, value):
         if isinstance(value, bytes_type):
             pass
@@ -673,7 +682,10 @@ class RequestHandler(object):
                     if inm and inm.find(etag) != -1:
                         self._write_buffer = []
                         self.set_status(304)
-            if "Content-Length" not in self._headers:
+            if self._status_code == 304:
+                assert not self._write_buffer, "Cannot send body with 304"
+                self._clear_headers_for_304()
+            elif "Content-Length" not in self._headers:
                 content_length = sum(len(part) for part in self._write_buffer)
                 self.set_header("Content-Length", content_length)
 
@@ -1065,6 +1077,17 @@ class RequestHandler(object):
     def _ui_method(self, method):
         return lambda *args, **kwargs: method(self, *args, **kwargs)
 
+    def _clear_headers_for_304(self):
+        # 304 responses should not contain entity headers (defined in
+        # http://www.w3.org/Protocols/rfc2616/rfc2616-sec7.html#sec7.1)
+        # not explicitly allowed by
+        # http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.5
+        headers = ["Allow", "Content-Encoding", "Content-Language",
+                   "Content-Length", "Content-MD5", "Content-Range",
+                   "Content-Type", "Last-Modified"]
+        for h in headers:
+            self.clear_header(h)
+
 
 def asynchronous(method):
     """Wrap request handler methods with this if they are asynchronous.
@@ -1729,7 +1752,9 @@ class ChunkedTransferEncoding(OutputTransform):
         self._chunking = request.supports_http_1_1()
 
     def transform_first_chunk(self, status_code, headers, chunk, finishing):
-        if self._chunking:
+        # 304 responses have no body (not even a zero-length body), and so
+        # should not have either Content-Length or Transfer-Encoding headers.
+        if self._chunking and status_code != 304:
             # No need to chunk the output if a Content-Length is specified
             if "Content-Length" in headers or "Transfer-Encoding" in headers:
                 self._chunking = False