Return HTTP 400 (Bad Request) on malformed requests

diff --git a/tornado/http1connection.py b/tornado/http1connection.py
index c6d3e336fbcf7ee1586e0dcea5fca29bf7041c14..6069e02785b3cb76497a83997a091fc0827d928a 100644 (file)
--- a/tornado/http1connection.py
+++ b/tornado/http1connection.py
@@ -250,6 +250,8 @@ class HTTP1Connection(httputil.HTTPConnection):
         except httputil.HTTPInputError as e:
             gen_log.info("Malformed HTTP message from %s: %s",
                          self.context, e)
+            if not self.is_client:
+                yield self.stream.write(b'HTTP/1.1 400 Bad Request\r\n\r\n')
             self.close()
             raise gen.Return(False)
         finally:

diff --git a/tornado/httputil.py b/tornado/httputil.py
old mode 100644 (file)
new mode 100755 (executable)
index 818ea91..5b87ce6
--- a/tornado/httputil.py
+++ b/tornado/httputil.py
@@ -829,6 +829,8 @@ def parse_request_start_line(line):
     try:
         method, path, version = line.split(" ")
     except ValueError:
+        # https://tools.ietf.org/html/rfc7230#section-3.1.1
+        # invalid request-line SHOULD respond with a 400 (Bad Request)
         raise HTTPInputError("Malformed HTTP request line")
     if not re.match(r"^HTTP/1\.[0-9]$", version):
         raise HTTPInputError(

diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py
index f5f91a9df2767a421726e79ba9bc646e954ef725..4169a43a7a47582d34357032a7afdde0ab8941c6 100644 (file)
--- a/tornado/test/httpserver_test.py
+++ b/tornado/test/httpserver_test.py
@@ -29,18 +29,19 @@ from io import BytesIO
 
 def read_stream_body(stream, callback):
     """Reads an HTTP response from `stream` and runs callback with its
-    headers and body."""
+    start_line, headers and body."""
     chunks = []
 
     class Delegate(HTTPMessageDelegate):
         def headers_received(self, start_line, headers):
             self.headers = headers
+            self.start_line = start_line
 
         def data_received(self, chunk):
             chunks.append(chunk)
 
         def finish(self):
-            callback((self.headers, b''.join(chunks)))
+            callback((self.start_line, self.headers, b''.join(chunks)))
     conn = HTTP1Connection(stream, True)
     conn.read_response(Delegate())
 
@@ -217,7 +218,7 @@ class HTTPConnectionTest(AsyncHTTPTestCase):
                              [utf8("Content-Length: %d" % len(body))]) +
                 newline + newline + body)
             read_stream_body(stream, self.stop)
-            headers, body = self.wait()
+            start_line, headers, body = self.wait()
             return body
 
     def test_multipart_form(self):
@@ -406,7 +407,15 @@ class HTTPServerRawTest(AsyncHTTPTestCase):
         self.io_loop.add_timeout(datetime.timedelta(seconds=0.001), self.stop)
         self.wait()
 
-    def test_malformed_first_line(self):
+    def test_malformed_first_line_response(self):
+        self.stream.write(b'asdf\r\n\r\n')
+        read_stream_body(self.stream, self.stop)
+        start_line, headers, response = self.wait()
+        self.assertEqual('HTTP/1.1', start_line.version)
+        self.assertEqual(400, start_line.code)
+        self.assertEqual('Bad Request', start_line.reason)
+
+    def test_malformed_first_line_log(self):
         with ExpectLog(gen_log, '.*Malformed HTTP request line'):
             self.stream.write(b'asdf\r\n\r\n')
             # TODO: need an async version of ExpectLog so we don't need
@@ -438,7 +447,7 @@ bar
 
 """.replace(b"\n", b"\r\n"))
         read_stream_body(self.stream, self.stop)
-        headers, response = self.wait()
+        start_line, headers, response = self.wait()
         self.assertEqual(json_decode(response), {u'foo': [u'bar']})
 
     def test_chunked_request_uppercase(self):
@@ -457,7 +466,7 @@ bar
 
 """.replace(b"\n", b"\r\n"))
         read_stream_body(self.stream, self.stop)
-        headers, response = self.wait()
+        start_line, headers, response = self.wait()
         self.assertEqual(json_decode(response), {u'foo': [u'bar']})
 
     def test_invalid_content_length(self):
@@ -627,7 +636,7 @@ class UnixSocketTest(AsyncTestCase):
             self.stream.write(b"garbage\r\n\r\n")
             self.stream.read_until_close(self.stop)
             response = self.wait()
-        self.assertEqual(response, b"")
+        self.assertEqual(response, b"HTTP/1.1 400 Bad Request\r\n\r\n")
 
 
 class KeepAliveTest(AsyncHTTPTestCase):
@@ -1036,24 +1045,26 @@ class BodyLimitsTest(AsyncHTTPTestCase):
     def test_large_body_buffered(self):
         with ExpectLog(gen_log, '.*Content-Length too long'):
             response = self.fetch('/buffered', method='PUT', body=b'a' * 10240)
-        self.assertEqual(response.code, 599)
+        self.assertEqual(response.code, 400)
 
     def test_large_body_buffered_chunked(self):
         with ExpectLog(gen_log, '.*chunked body too large'):
             response = self.fetch('/buffered', method='PUT',
                                   body_producer=lambda write: write(b'a' * 10240))
-        self.assertEqual(response.code, 599)
+        # this test is flaky on windows; accept 400 (expected) or 599
+        self.assertIn(response.code, [400, 599])
 
     def test_large_body_streaming(self):
         with ExpectLog(gen_log, '.*Content-Length too long'):
             response = self.fetch('/streaming', method='PUT', body=b'a' * 10240)
-        self.assertEqual(response.code, 599)
+        self.assertEqual(response.code, 400)
 
     def test_large_body_streaming_chunked(self):
         with ExpectLog(gen_log, '.*chunked body too large'):
             response = self.fetch('/streaming', method='PUT',
                                   body_producer=lambda write: write(b'a' * 10240))
-        self.assertEqual(response.code, 599)
+        # this test is flaky on windows; accept 400 (expected) or 599
+        self.assertIn(response.code, [400, 599])
 
     def test_large_body_streaming_override(self):
         response = self.fetch('/streaming?expected_size=10240', method='PUT',
@@ -1090,14 +1101,14 @@ class BodyLimitsTest(AsyncHTTPTestCase):
             stream.write(b'PUT /streaming?expected_size=10240 HTTP/1.1\r\n'
                          b'Content-Length: 10240\r\n\r\n')
             stream.write(b'a' * 10240)
-            headers, response = yield gen.Task(read_stream_body, stream)
+            start_line, headers, response = yield gen.Task(read_stream_body, stream)
             self.assertEqual(response, b'10240')
             # Without the ?expected_size parameter, we get the old default value
             stream.write(b'PUT /streaming HTTP/1.1\r\n'
                          b'Content-Length: 10240\r\n\r\n')
             with ExpectLog(gen_log, '.*Content-Length too long'):
                 data = yield stream.read_until_close()
-            self.assertEqual(data, b'')
+            self.assertEqual(data, b'HTTP/1.1 400 Bad Request\r\n\r\n')
         finally:
             stream.close()