Repository: tornadoweb/tornado
Pull request #1743 (2016-06-08 01:49:07)

Allow 204 responses without Content-Length/Transfer-Encoding
These changes relate to #1736:

* http1connection now also considers 204 to decide when chunking isn't necessary
* RequestHandler now also asserts the write buffer is empty for 204 responses


Patch:
diff --git a/tornado/http1connection.py b/tornado/http1connection.py
index 43034c6b..d0e91b82 100644
--- a/tornado/http1connection.py
+++ b/tornado/http1connection.py
@@ -351,7 +351,7 @@ class HTTP1Connection(httputil.HTTPConnection):
                 # 304 responses have no body (not even a zero-length body), and so
                 # should not have either Content-Length or Transfer-Encoding.
                 # headers.
-                start_line.code != 304 and
+                start_line.code not in (204, 304) and
                 # No need to chunk the output if a Content-Length is specified.
                 'Content-Length' not in headers and
                 # Applications are discouraged from touching Transfer-Encoding,
diff --git a/tornado/web.py b/tornado/web.py
index 254a91ac..479dd245 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -935,8 +935,8 @@ class RequestHandler(object):
                 if self.check_etag_header():
                     self._write_buffer = []
                     self.set_status(304)
-            if self._status_code == 304:
-                assert not self._write_buffer, "Cannot send body with 304"
+            if self._status_code in (204, 304):
+                assert not self._write_buffer, "Cannot send body with %s" % self._status_code
                 self._clear_headers_for_304()
             elif "Content-Length" not in self._headers:
                 content_length = sum(len(part) for part in self._write_buffer)


Test patch:
diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py
index 74073182..a5624af2 100644
--- a/tornado/test/simple_httpclient_test.py
+++ b/tornado/test/simple_httpclient_test.py
@@ -73,10 +73,8 @@ class OptionsHandler(RequestHandler):
 
 class NoContentHandler(RequestHandler):
     def get(self):
-        if self.get_argument("error", None):
-            self.set_header("Content-Length", "5")
-            self.write("hello")
         self.set_status(204)
+        self.finish()
 
 
 class SeeOtherPostHandler(RequestHandler):
@@ -322,17 +320,11 @@ class SimpleHTTPClientTestMixin(object):
     def test_no_content(self):
         response = self.fetch("/no_content")
         self.assertEqual(response.code, 204)
-        # 204 status doesn't need a content-length, but tornado will
-        # add a zero content-length anyway.
+        # 204 status shouldn't have a content-length
         #
-        # A test without a content-length header is included below
+        # Tests with a content-length header are included below
         # in HTTP204NoContentTestCase.
-        self.assertEqual(response.headers["Content-length"], "0")
-
-        # 204 status with non-zero content length is malformed
-        with ExpectLog(gen_log, "Malformed HTTP message"):
-            response = self.fetch("/no_content?error=1")
-        self.assertEqual(response.code, 599)
+        self.assertNotIn("Content-Length", response.headers)
 
     def test_host_header(self):
         host_re = re.compile(b"^localhost:[0-9]+$")
@@ -611,16 +603,20 @@ class HTTP204NoContentTestCase(AsyncHTTPTestCase):
                                              HTTPHeaders())
             request.connection.finish()
             return
+
         # A 204 response never has a body, even if doesn't have a content-length
-        # (which would otherwise mean read-until-close).  Tornado always
-        # sends a content-length, so we simulate here a server that sends
-        # no content length and does not close the connection.
+        # (which would otherwise mean read-until-close).  We simulate here a
+        # server that sends no content length and does not close the connection.
         #
-        # Tests of a 204 response with a Content-Length header are included
+        # Tests of a 204 response with no Content-Length header are included
         # in SimpleHTTPClientTestMixin.
         stream = request.connection.detach()
-        stream.write(
-            b"HTTP/1.1 204 No content\r\n\r\n")
+        stream.write(b"HTTP/1.1 204 No content\r\n")
+        if request.arguments.get("error", [False])[-1]:
+            stream.write(b"Content-Length: 5\r\n")
+        else:
+            stream.write(b"Content-Length: 0\r\n")
+        stream.write(b"\r\n")
         stream.close()
 
     def get_app(self):
@@ -633,6 +629,15 @@ class HTTP204NoContentTestCase(AsyncHTTPTestCase):
         self.assertEqual(resp.code, 204)
         self.assertEqual(resp.body, b'')
 
+    def test_204_invalid_content_length(self):
+        # 204 status with non-zero content length is malformed
+        response = self.fetch("/?error=1")
+        if not self.http1:
+            self.skipTest("requires HTTP/1.x")
+        if self.http_client.configured_class != SimpleAsyncHTTPClient:
+            self.skipTest("curl client accepts invalid headers")
+        self.assertEqual(response.code, 599)
+
 
 class HostnameMappingTestCase(AsyncHTTPTestCase):
     def setUp(self):
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 744803a1..745d0fa8 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -1397,6 +1397,19 @@ class ClearHeaderTest(SimpleHandlerTestCase):
         self.assertEqual(response.headers["h2"], "bar")
 
 
+class Header204Test(SimpleHandlerTestCase):
+    class Handler(RequestHandler):
+        def get(self):
+            self.set_status(204)
+            self.finish()
+
+    def test_204_headers(self):
+        response = self.fetch('/')
+        self.assertEqual(response.code, 204)
+        self.assertNotIn("Content-Length", response.headers)
+        self.assertNotIn("Transfer-Encoding", response.headers)
+
+
 @wsgi_safe
 class Header304Test(SimpleHandlerTestCase):
     class Handler(RequestHandler):