Fixed #150: Correctly handle bad querystrings

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,

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.

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"))