Skip to content

Commit

Permalink
Improve error messages on HTTP parsing errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
aaugustin committed Sep 8, 2024
1 parent 560f6ee commit f618012
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 23 deletions.
32 changes: 19 additions & 13 deletions src/websockets/http11.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,17 @@ 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: {d(request_line)}"
)
path = raw_path.decode("ascii", "surrogateescape")

headers = yield from parse_headers(read_line)
Expand Down Expand Up @@ -236,23 +239,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: {d(status_line)}"
) from None
if not 100 <= status_code < 1000:
raise ValueError(f"unsupported HTTP status code: {d(raw_status_code)}")
raise ValueError(
f"invalid status code; expected 3-digit integer: {d(status_line)}"
)
if not _value_re.fullmatch(raw_reason):
raise ValueError(f"invalid HTTP reason phrase: {d(raw_reason)}")
reason = raw_reason.decode()
raise ValueError(f"invalid HTTP reason phrase: {d(status_line)}")
reason = raw_reason.decode("ascii", "surrogateescape")

headers = yield from parse_headers(read_line)

Expand Down
3 changes: 2 additions & 1 deletion tests/asyncio/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)


Expand Down
3 changes: 2 additions & 1 deletion tests/sync/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
17 changes: 9 additions & 8 deletions tests/test_http11.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def test_parse_unsupported_method(self):
next(self.parse())
self.assertEqual(
str(raised.exception),
"unsupported HTTP method: OPTIONS",
"unsupported HTTP method; expected GET: OPTIONS * HTTP/1.1",
)

def test_parse_unsupported_version(self):
Expand All @@ -65,7 +65,7 @@ def test_parse_unsupported_version(self):
next(self.parse())
self.assertEqual(
str(raised.exception),
"unsupported HTTP version: HTTP/1.0",
"unsupported protocol; expected HTTP/1.1: GET /chat HTTP/1.0",
)

def test_parse_invalid_header(self):
Expand Down Expand Up @@ -177,25 +177,26 @@ def test_parse_unsupported_version(self):
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: HTTP/1.1 OMG WTF",
)

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",
"invalid status code; expected 3-digit integer: "
"HTTP/1.1 007 My name is Bond",
)

def test_parse_invalid_reason(self):
Expand All @@ -204,7 +205,7 @@ def test_parse_invalid_reason(self):
next(self.parse())
self.assertEqual(
str(raised.exception),
"invalid HTTP reason phrase: \x7f",
"invalid HTTP reason phrase: HTTP/1.1 200 \x7f",
)

def test_parse_invalid_header(self):
Expand Down

0 comments on commit f618012

Please sign in to comment.