diff --git a/CHANGES/7366.feature b/CHANGES/7366.feature new file mode 100644 index 00000000000..8e38f70f898 --- /dev/null +++ b/CHANGES/7366.feature @@ -0,0 +1 @@ +Added information to C parser exceptions to show which character caused the error. -- by :user:`Dreamsorcerer` diff --git a/aiohttp/_http_parser.pyx b/aiohttp/_http_parser.pyx index bebd9894374..4f39dd0c978 100644 --- a/aiohttp/_http_parser.pyx +++ b/aiohttp/_http_parser.pyx @@ -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 - 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 @@ -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) @@ -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)) diff --git a/aiohttp/http_exceptions.py b/aiohttp/http_exceptions.py index d49c59e08f1..728824f856f 100644 --- a/aiohttp/http_exceptions.py +++ b/aiohttp/http_exceptions.py @@ -1,6 +1,7 @@ """Low-level http related exceptions.""" +from textwrap import indent from typing import Optional, Union from .typedefs import _CIMultiDict @@ -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): diff --git a/tests/test_http_exceptions.py b/tests/test_http_exceptions.py index 27aab67089f..28fdcbe0c69 100644 --- a/tests/test_http_exceptions.py +++ b/tests/test_http_exceptions.py @@ -32,13 +32,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) == ("") + assert repr(err) == ("") class TestBadHttpMessage: @@ -61,7 +61,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={}) @@ -88,9 +88,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") @@ -120,25 +119,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) == ( - "" - ) + expected = "" + 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") diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 7d7ac1f9d15..8113fb94dd1 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -2,6 +2,7 @@ # Tests for aiohttp/protocol.py import asyncio +import re from typing import Any, List from unittest import mock from urllib.parse import quote @@ -117,6 +118,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_headers_longline(parser: Any) -> None: invalid_unicode_byte = b"\xd9" header_name = b"Test" + invalid_unicode_byte + b"Header" + b"A" * 8192 @@ -436,7 +457,7 @@ def test_max_header_field_size(parser: Any, size: Any) -> 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) @@ -464,7 +485,7 @@ def test_max_header_value_size(parser: Any, size: Any) -> 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) @@ -492,7 +513,7 @@ def test_max_header_value_size_continuation(parser: Any, size: Any) -> 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) @@ -615,7 +636,7 @@ def test_http_request_parser_bad_version(parser: Any) -> None: @pytest.mark.parametrize("size", [40965, 8191]) def test_http_request_max_status_line(parser: Any, size: Any) -> 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") @@ -660,7 +681,7 @@ def test_http_response_parser_bad_status_line_too_long( response: Any, size: Any ) -> 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")