Skip to content

Commit

Permalink
Improve error messages from C parser (#7366)
Browse files Browse the repository at this point in the history
(cherry picked from commit 1a48add)
  • Loading branch information
Dreamsorcerer committed Jul 18, 2023
1 parent 9337fb3 commit 2d0c251
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGES/7366.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added information to C parser exceptions to show which character caused the error. -- by :user:`Dreamsorcerer`
12 changes: 9 additions & 3 deletions aiohttp/_http_parser.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,13 @@ cdef class HttpParser:
ex = self._last_error
self._last_error = None
else:
ex = parser_error_from_errno(self._cparser)
after = cparser.llhttp_get_error_pos(self._cparser)
before = data[:after - <char*>self.py_buf.buf]
after_b = after.split(b"\n", 1)[0]
before = before.rsplit(b"\n", 1)[-1]
data = before + after_b
pointer = " " * (len(repr(before))-1) + "^"
ex = parser_error_from_errno(self._cparser, data, pointer)
self._payload = None
raise ex

Expand Down Expand Up @@ -797,7 +803,7 @@ cdef int cb_on_chunk_complete(cparser.llhttp_t* parser) except -1:
return 0


cdef parser_error_from_errno(cparser.llhttp_t* parser):
cdef parser_error_from_errno(cparser.llhttp_t* parser, data, pointer):
cdef cparser.llhttp_errno_t errno = cparser.llhttp_get_errno(parser)
cdef bytes desc = cparser.llhttp_get_error_reason(parser)

Expand Down Expand Up @@ -829,4 +835,4 @@ cdef parser_error_from_errno(cparser.llhttp_t* parser):
else:
cls = BadHttpMessage

return cls(desc.decode('latin-1'))
return cls("{}:\n\n {!r}\n {}".format(desc.decode("latin-1"), data, pointer))
6 changes: 4 additions & 2 deletions aiohttp/http_exceptions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Low-level http related exceptions."""


from textwrap import indent
from typing import Optional, Union

from .typedefs import _CIMultiDict
Expand Down Expand Up @@ -35,10 +36,11 @@ def __init__(
self.message = message

def __str__(self) -> str:
return f"{self.code}, message={self.message!r}"
msg = indent(self.message, " ")
return f"{self.code}, message:\n{msg}"

def __repr__(self) -> str:
return f"<{self.__class__.__name__}: {self}>"
return f"<{self.__class__.__name__}: {self.code}, message={self.message!r}>"


class BadHttpMessage(HttpProcessingError):
Expand Down
22 changes: 10 additions & 12 deletions tests/test_http_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ def test_str(self) -> None:
err = http_exceptions.HttpProcessingError(
code=500, message="Internal error", headers={}
)
assert str(err) == "500, message='Internal error'"
assert str(err) == "500, message:\n Internal error"

def test_repr(self) -> None:
err = http_exceptions.HttpProcessingError(
code=500, message="Internal error", headers={}
)
assert repr(err) == ("<HttpProcessingError: 500, " "message='Internal error'>")
assert repr(err) == ("<HttpProcessingError: 500, message='Internal error'>")


class TestBadHttpMessage:
Expand All @@ -60,7 +60,7 @@ def test_pickle(self) -> None:

def test_str(self) -> None:
err = http_exceptions.BadHttpMessage(message="Bad HTTP message", headers={})
assert str(err) == "400, message='Bad HTTP message'"
assert str(err) == "400, message:\n Bad HTTP message"

def test_repr(self) -> None:
err = http_exceptions.BadHttpMessage(message="Bad HTTP message", headers={})
Expand All @@ -87,9 +87,8 @@ def test_pickle(self) -> None:

def test_str(self) -> None:
err = http_exceptions.LineTooLong(line="spam", limit="10", actual_size="12")
assert str(err) == (
"400, message='Got more than 10 bytes (12) " "when reading spam.'"
)
expected = "400, message:\n Got more than 10 bytes (12) when reading spam."
assert str(err) == expected

def test_repr(self) -> None:
err = http_exceptions.LineTooLong(line="spam", limit="10", actual_size="12")
Expand Down Expand Up @@ -119,25 +118,24 @@ def test_pickle(self) -> None:

def test_str(self) -> None:
err = http_exceptions.InvalidHeader(hdr="X-Spam")
assert str(err) == "400, message='Invalid HTTP Header: X-Spam'"
assert str(err) == "400, message:\n Invalid HTTP Header: X-Spam"

def test_repr(self) -> None:
err = http_exceptions.InvalidHeader(hdr="X-Spam")
assert repr(err) == (
"<InvalidHeader: 400, " "message='Invalid HTTP Header: X-Spam'>"
)
expected = "<InvalidHeader: 400, message='Invalid HTTP Header: X-Spam'>"
assert repr(err) == expected


class TestBadStatusLine:
def test_ctor(self) -> None:
err = http_exceptions.BadStatusLine("Test")
assert err.line == "Test"
assert str(err) == "400, message=\"Bad status line 'Test'\""
assert str(err) == "400, message:\n Bad status line 'Test'"

def test_ctor2(self) -> None:
err = http_exceptions.BadStatusLine(b"")
assert err.line == "b''"
assert str(err) == "400, message='Bad status line \"b\\'\\'\"'"
assert str(err) == "400, message:\n Bad status line \"b''\""

def test_pickle(self) -> None:
err = http_exceptions.BadStatusLine("Test")
Expand Down
31 changes: 26 additions & 5 deletions tests/test_http_parser.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Tests for aiohttp/protocol.py

import asyncio
import re
from typing import Any, List
from unittest import mock
from urllib.parse import quote
Expand Down Expand Up @@ -118,6 +119,26 @@ def test_parse_headers(parser: Any) -> None:
assert not msg.upgrade


@pytest.mark.skipif(NO_EXTENSIONS, reason="Only tests C parser.")
def test_invalid_character(loop: Any, protocol: Any, request: Any) -> None:
parser = HttpRequestParserC(
protocol,
loop,
2**16,
max_line_size=8190,
max_field_size=8190,
)
text = b"POST / HTTP/1.1\r\nHost: localhost:8080\r\nSet-Cookie: abc\x01def\r\n\r\n"
error_detail = re.escape(
r""":
b'Set-Cookie: abc\x01def\r'
^"""
)
with pytest.raises(http_exceptions.BadHttpMessage, match=error_detail):
parser.feed_data(text)


def test_parse(parser) -> None:
text = b"GET /test HTTP/1.1\r\n\r\n"
messages, upgrade, tail = parser.feed_data(text)
Expand Down Expand Up @@ -429,7 +450,7 @@ def test_max_header_field_size(parser, size) -> None:
name = b"t" * size
text = b"GET /test HTTP/1.1\r\n" + name + b":data\r\n\r\n"

match = f"400, message='Got more than 8190 bytes \\({size}\\) when reading"
match = f"400, message:\n Got more than 8190 bytes \\({size}\\) when reading"
with pytest.raises(http_exceptions.LineTooLong, match=match):
parser.feed_data(text)

Expand Down Expand Up @@ -457,7 +478,7 @@ def test_max_header_value_size(parser, size) -> None:
name = b"t" * size
text = b"GET /test HTTP/1.1\r\n" b"data:" + name + b"\r\n\r\n"

match = f"400, message='Got more than 8190 bytes \\({size}\\) when reading"
match = f"400, message:\n Got more than 8190 bytes \\({size}\\) when reading"
with pytest.raises(http_exceptions.LineTooLong, match=match):
parser.feed_data(text)

Expand Down Expand Up @@ -485,7 +506,7 @@ def test_max_header_value_size_continuation(parser, size) -> None:
name = b"T" * (size - 5)
text = b"GET /test HTTP/1.1\r\n" b"data: test\r\n " + name + b"\r\n\r\n"

match = f"400, message='Got more than 8190 bytes \\({size}\\) when reading"
match = f"400, message:\n Got more than 8190 bytes \\({size}\\) when reading"
with pytest.raises(http_exceptions.LineTooLong, match=match):
parser.feed_data(text)

Expand Down Expand Up @@ -608,7 +629,7 @@ def test_http_request_parser_bad_version(parser) -> None:
@pytest.mark.parametrize("size", [40965, 8191])
def test_http_request_max_status_line(parser, size) -> None:
path = b"t" * (size - 5)
match = f"400, message='Got more than 8190 bytes \\({size}\\) when reading"
match = f"400, message:\n Got more than 8190 bytes \\({size}\\) when reading"
with pytest.raises(http_exceptions.LineTooLong, match=match):
parser.feed_data(b"GET /path" + path + b" HTTP/1.1\r\n\r\n")

Expand Down Expand Up @@ -651,7 +672,7 @@ def test_http_response_parser_utf8(response) -> None:
@pytest.mark.parametrize("size", [40962, 8191])
def test_http_response_parser_bad_status_line_too_long(response, size) -> None:
reason = b"t" * (size - 2)
match = f"400, message='Got more than 8190 bytes \\({size}\\) when reading"
match = f"400, message:\n Got more than 8190 bytes \\({size}\\) when reading"
with pytest.raises(http_exceptions.LineTooLong, match=match):
response.feed_data(b"HTTP/1.1 200 Ok" + reason + b"\r\n\r\n")

Expand Down

0 comments on commit 2d0c251

Please sign in to comment.