Improve error messages on HTTP parsing errors.

Author: aaugustin

src/websockets/http11.py

@@ -135,14 +135,15 @@ def parse(
             raise EOFError("connection closed while reading HTTP request line") from exc
 
         try:
-            method, raw_path, version = request_line.split(b" ", 2)
+            method, raw_path, protocol = request_line.split(b" ", 2)
         except ValueError:  # not enough values to unpack (expected 3, got 1-2)
             raise ValueError(f"invalid HTTP request line: {d(request_line)}") from None
-
+        if protocol != b"HTTP/1.1":
+            raise ValueError(
+                f"unsupported protocol; expected HTTP/1.1: {d(request_line)}"
+            )
         if method != b"GET":
-            raise ValueError(f"unsupported HTTP method: {d(method)}")
-        if version != b"HTTP/1.1":
-            raise ValueError(f"unsupported HTTP version: {d(version)}")
+            raise ValueError(f"unsupported HTTP method; expected GET; got {d(method)}")
         path = raw_path.decode("ascii", "surrogateescape")
 
         headers = yield from parse_headers(read_line)
@@ -236,23 +237,26 @@ def parse(
             raise EOFError("connection closed while reading HTTP status line") from exc
 
         try:
-            version, raw_status_code, raw_reason = status_line.split(b" ", 2)
+            protocol, raw_status_code, raw_reason = status_line.split(b" ", 2)
         except ValueError:  # not enough values to unpack (expected 3, got 1-2)
             raise ValueError(f"invalid HTTP status line: {d(status_line)}") from None
-
-        if version != b"HTTP/1.1":
-            raise ValueError(f"unsupported HTTP version: {d(version)}")
+        if protocol != b"HTTP/1.1":
+            raise ValueError(
+                f"unsupported protocol; expected HTTP/1.1: {d(status_line)}"
+            )
         try:
             status_code = int(raw_status_code)
         except ValueError:  # invalid literal for int() with base 10
             raise ValueError(
-                f"invalid HTTP status code: {d(raw_status_code)}"
+                f"invalid status code; expected integer; got {d(raw_status_code)}"
             ) from None
-        if not 100 <= status_code < 1000:
-            raise ValueError(f"unsupported HTTP status code: {d(raw_status_code)}")
+        if not 100 <= status_code < 600:
+            raise ValueError(
+                f"invalid status code; expected 100–599; got {d(raw_status_code)}"
+            )
         if not _value_re.fullmatch(raw_reason):
             raise ValueError(f"invalid HTTP reason phrase: {d(raw_reason)}")
-        reason = raw_reason.decode()
+        reason = raw_reason.decode("ascii", "surrogateescape")
 
         headers = yield from parse_headers(read_line)
 

tests/asyncio/test_client.py

@@ -426,7 +426,8 @@ async def junk(reader, writer):
                     self.fail("did not raise")
             self.assertEqual(
                 str(raised.exception),
-                "unsupported HTTP version: 220",
+                "unsupported protocol; expected HTTP/1.1: "
+                "220 smtp.invalid ESMTP Postfix",
             )
 
 

tests/sync/test_client.py

@@ -177,7 +177,8 @@ def handle(self):
                         self.fail("did not raise")
                 self.assertEqual(
                     str(raised.exception),
-                    "unsupported HTTP version: 220",
+                    "unsupported protocol; expected HTTP/1.1: "
+                    "220 smtp.invalid ESMTP Postfix",
                 )
             finally:
                 server.shutdown()

tests/test_http11.py

@@ -50,22 +50,22 @@ def test_parse_invalid_request_line(self):
             "invalid HTTP request line: GET /",
         )
 
-    def test_parse_unsupported_method(self):
-        self.reader.feed_data(b"OPTIONS * HTTP/1.1\r\n\r\n")
+    def test_parse_unsupported_protocol(self):
+        self.reader.feed_data(b"GET /chat HTTP/1.0\r\n\r\n")
         with self.assertRaises(ValueError) as raised:
             next(self.parse())
         self.assertEqual(
             str(raised.exception),
-            "unsupported HTTP method: OPTIONS",
+            "unsupported protocol; expected HTTP/1.1: GET /chat HTTP/1.0",
         )
 
-    def test_parse_unsupported_version(self):
-        self.reader.feed_data(b"GET /chat HTTP/1.0\r\n\r\n")
+    def test_parse_unsupported_method(self):
+        self.reader.feed_data(b"OPTIONS * HTTP/1.1\r\n\r\n")
         with self.assertRaises(ValueError) as raised:
             next(self.parse())
         self.assertEqual(
             str(raised.exception),
-            "unsupported HTTP version: HTTP/1.0",
+            "unsupported HTTP method; expected GET; got OPTIONS",
         )
 
     def test_parse_invalid_header(self):
@@ -171,31 +171,30 @@ def test_parse_invalid_status_line(self):
             "invalid HTTP status line: Hello!",
         )
 
-    def test_parse_unsupported_version(self):
+    def test_parse_unsupported_protocol(self):
         self.reader.feed_data(b"HTTP/1.0 400 Bad Request\r\n\r\n")
         with self.assertRaises(ValueError) as raised:
             next(self.parse())
         self.assertEqual(
             str(raised.exception),
-            "unsupported HTTP version: HTTP/1.0",
+            "unsupported protocol; expected HTTP/1.1: HTTP/1.0 400 Bad Request",
         )
 
-    def test_parse_invalid_status(self):
+    def test_parse_non_integer_status(self):
         self.reader.feed_data(b"HTTP/1.1 OMG WTF\r\n\r\n")
         with self.assertRaises(ValueError) as raised:
             next(self.parse())
         self.assertEqual(
             str(raised.exception),
-            "invalid HTTP status code: OMG",
+            "invalid status code; expected integer; got OMG",
         )
 
-    def test_parse_unsupported_status(self):
+    def test_parse_non_three_digit_status(self):
         self.reader.feed_data(b"HTTP/1.1 007 My name is Bond\r\n\r\n")
         with self.assertRaises(ValueError) as raised:
             next(self.parse())
         self.assertEqual(
-            str(raised.exception),
-            "unsupported HTTP status code: 007",
+            str(raised.exception), "invalid status code; expected 100–599; got 007"
         )
 
     def test_parse_invalid_reason(self):