From: Jehiah Czebotar Date: Fri, 26 May 2017 13:15:26 +0000 (-0400) Subject: Return HTTP 400 (Bad Request) on malformed requests X-Git-Tag: v5.0.0~75^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=30dd0a7ad73dcef44f5d1bce5a6111784f473df3;p=thirdparty%2Ftornado.git Return HTTP 400 (Bad Request) on malformed requests --- diff --git a/tornado/http1connection.py b/tornado/http1connection.py index c6d3e336f..6069e0278 100644 --- 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 new mode 100755 index 818ea914c..5b87ce610 --- 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 f5f91a9df..4169a43a7 100644 --- 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()