8000 Improve error messages from C parser (#7366) (#7380) · aio-libs/aiohttp@135a45e · GitHub
[go: up one dir, main page]
Skip to content

Commit 135a45e

Browse files
DreamsorcererDreamsorcerer
authored
Improve error messages from C parser (#7366) (#7380)
(cherry picked from commit 1a48add)
1 parent 9337fb3 commit 135a45e

File tree

5 files changed

+50
-22
lines changed

5 files changed

+50
-22
lines changed

‎CHANGES/7366.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Added information to C parser exceptions to show which character caused the error. -- by :user:`Dreamsorcerer`

‎aiohttp/_http_parser.pyx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,13 @@ cdef class HttpParser:
546546
ex = self._last_error
547547
self._last_error = None
548548
else:
549-
ex = parser_error_from_errno(self._cparser)
549+
after = cparser.llhttp_get_error_pos(self._cparser)
550+
before = data[:after - <char*>self.py_buf.buf]
551+
after_b = after.split(b"\n", 1)[0]
552+
before = before.rsplit(b"\n", 1)[-1]
553+
data = before + after_b
554+
pointer = " " * (len(repr(before))-1) + "^"
555+
ex = parser_error_from_errno(self._cparser, data, pointer)
550556
self._payload = None
551557
raise ex
552558

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

799805

800-
cdef parser_error_from_errno(cparser.llhttp_t* parser):
806+
cdef parser_error_from_errno(cparser.llhttp_t* parser, data, pointer):
801807
cdef cparser.llhttp_errno_t errno = cparser.llhttp_get_errno(parser)
802808
cdef bytes desc = cparser.llhttp_get_error_reason(parser)
803809

@@ -829,4 +835,4 @@ cdef parser_error_from_errno(cparser.llhttp_t* parser):
829835
else:
830836
cls = BadHttpMessage
831837

832-
return cls(desc.decode('latin-1'))
838+
return cls("{}:\n\n {!r}\n {}".format(desc.decode("latin-1"), data, pointer))

‎aiohttp/http_exceptions.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Low-level http related exceptions."""
22

33

4+
from textwrap import indent
45
from typing import Optional, Union
56

67
from .typedefs import _CIMultiDict
@@ -35,10 +36,11 @@ def __init__(
3536
self.message = message
3637

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

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

4345

4446
class BadHttpMessage(HttpProcessingError):

‎tests/test_http_exceptions.py

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ def test_str(self) -> None:
3131
err = http_exceptions.HttpProcessingError(
3232
code=500, message="Internal error", headers={}
3333
)
34-
assert str(err) == "500, message='Internal error'"
34+
assert str(err) == "500, message:\n Internal error"
3535

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

4242

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

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

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

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

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

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

124123
def test_repr(self) -> None:
125124
err = http_exceptions.InvalidHeader(hdr="X-Spam")
126-
assert repr(err) == (
127-
"<InvalidHeader: 400, " "message='Invalid HTTP Header: X-Spam'>"
128-
)
125+
expected = "<InvalidHeader: 400, message='Invalid HTTP Header: X-Spam'>"
126+
assert repr(err) == expected
129127

130128

131129
class TestBadStatusLine:
132130
def test_ctor(self) -> None:
133131
err = http_exceptions.BadStatusLine("Test")
134132
assert err.line == "Test"
135-
assert str(err) == "400, message=\"Bad status line 'Test'\""
133+
assert str(err) == "400, message:\n Bad status line 'Test'"
136134

137135
def test_ctor2(self) -> None:
138136
err = http_exceptions.BadStatusLine(b"")
139137
assert err.line == "b''"
140-
assert str(err) == "400, message='Bad status line \"b\\'\\'\"'"
138+
assert str(err) == "400, message:\n Bad status line \"b''\""
141139

142140
def test_pickle(self) -> None:
143141
err = http_exceptions.BadStatusLine("Test")

‎tests/test_http_parser.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Tests for aiohttp/protocol.py
22

33
import asyncio
4+
import re
45
from typing import Any, List
56
from unittest import mock
67
from urllib.parse import quote
@@ -118,6 +119,26 @@ def test_parse_headers(parser: Any) -> None:
118119
assert not msg.upgrade
119120

120121

122+
@pytest.mark.skipif(NO_EXTENSIONS, reason="Only tests C parser.")
123+
def test_invalid_character(loop: Any, protocol: Any, request: Any) -> None:
124+
parser = HttpRequestParserC(
125+
protocol,
126+
loop,
127+
2**16,
128+
max_line_size=8190,
129+
max_field_size=8190,
130+
)
131+
text = b"POST / HTTP/1.1\r\nHost: localhost:8080\r\nSet-Cookie: abc\x01def\r\n\r\n"
132+
error_detail = re.escape(
133+
r""":
134+
135+
b'Set-Cookie: abc\x01def\r'
136+
^"""
137+
)
138+
with pytest.raises(http_exceptions.BadHttpMessage, match=error_detail):
139+
parser.feed_data(text)
140+
141+
121142
def test_parse(parser) -> None:
122143
text = b"GET /test HTTP/1.1\r\n\r\n"
123144
messages, upgrade, tail = parser.feed_data(text)
@@ -429,7 +450,7 @@ def test_max_header_field_size(parser, size) -> None:
429450
name = b"t" * size
430451
text = b"GET /test HTTP/1.1\r\n" + name + b":data\r\n\r\n"
431452

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

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

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

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

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

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

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

0 commit comments

Comments
 (0)
0