From 446fc69408c07286af7cfff9edc61003d849966b Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Sun, 4 Mar 2018 09:48:33 -0800 Subject: [PATCH] Fixed #150: Correctly handle bad querystrings --- daphne/http_protocol.py | 14 ++++++++++++-- tests/http_base.py | 16 ++++++++++++++++ tests/test_http_request.py | 11 +++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/daphne/http_protocol.py b/daphne/http_protocol.py index 97a28a0..9aa5960 100755 --- a/daphne/http_protocol.py +++ b/daphne/http_protocol.py @@ -90,6 +90,11 @@ class WebRequest(http.Request): self.query_string = b"" if b"?" in self.uri: self.query_string = self.uri.split(b"?", 1)[1] + try: + self.query_string.decode("ascii") + except UnicodeDecodeError: + self.basic_error(400, b"Bad Request", "Invalid query string") + return # Is it WebSocket? IS IT?! if upgrade_header and upgrade_header.lower() == b"websocket": # Make WebSocket protocol to hand off to @@ -217,11 +222,16 @@ class WebRequest(http.Request): if not message.get("more_body", False): self.finish() logger.debug("HTTP response complete for %s", self.client_addr) + try: + uri = self.uri.decode("ascii") + except UnicodeDecodeError: + # The path is malformed somehow - do our best to log something + uri = repr(self.uri) try: self.server.log_action("http", "complete", { - "path": self.uri.decode("ascii"), + "path": uri, "status": self.code, - "method": self.method.decode("ascii"), + "method": self.method.decode("ascii", "replace"), "client": "%s:%s" % tuple(self.client_addr) if self.client_addr else None, "time_taken": self.duration(), "size": self.sentLength, diff --git a/tests/http_base.py b/tests/http_base.py index 2af0e26..c7cec48 100644 --- a/tests/http_base.py +++ b/tests/http_base.py @@ -54,6 +54,22 @@ class DaphneTestCase(unittest.TestCase): # Return scope, messages, response return test_app.get_received() + (response, ) + def run_daphne_raw(self, data, timeout=1): + """ + Runs daphne and sends it the given raw bytestring over a socket. Returns what it sends back. + """ + assert isinstance(data, bytes) + with DaphneTestingInstance() as test_app: + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + s.settimeout(timeout) + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + s.connect((test_app.host, test_app.port)) + s.send(data) + try: + return s.recv(1000000) + except socket.timeout: + raise RuntimeError("Daphne timed out handling raw request, no exception found.") + def run_daphne_request(self, method, path, params=None, body=None, headers=None, xff=False): """ Convenience method for just testing request handling. diff --git a/tests/test_http_request.py b/tests/test_http_request.py index d2fd1ee..79274ed 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -259,3 +259,14 @@ class TestHTTPRequest(DaphneTestCase): self.assert_valid_http_request_message(messages[0], body=b"") # It should now appear in the client scope item self.assertEqual(scope["client"], ["10.1.2.3", 0]) + + def test_bad_requests(self): + """ + Tests that requests with invalid (non-ASCII) characters fail. + """ + # Bad path + response = self.run_daphne_raw(b"GET /\xc3\xa4\xc3\xb6\xc3\xbc HTTP/1.0\r\n\r\n") + self.assertTrue(response.startswith(b"HTTP/1.0 400 Bad Request")) + # Bad querystring + response = self.run_daphne_raw(b"GET /?\xc3\xa4\xc3\xb6\xc3\xbc HTTP/1.0\r\n\r\n") + self.assertTrue(response.startswith(b"HTTP/1.0 400 Bad Request"))