Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/13203.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a segmentation fault in the C HTTP parser on Python 3.12 and newer when payload decompression raised an error while pending decompressed data was being drained, as seen with ``brotlicffi`` 1.2 -- by :user:`bdraco`.
1 change: 1 addition & 0 deletions CHANGES/13249.bugfix.rst
4 changes: 2 additions & 2 deletions aiohttp/_cparser.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ cdef extern from "llhttp.h":
ctypedef llhttp__internal_s llhttp__internal_t
ctypedef llhttp__internal_t llhttp_t

ctypedef int (*llhttp_data_cb)(llhttp_t*, const char *at, size_t length) except -1
ctypedef int (*llhttp_cb)(llhttp_t*) except -1
ctypedef int (*llhttp_data_cb)(llhttp_t*, const char *at, size_t length) except? -1
ctypedef int (*llhttp_cb)(llhttp_t*) except? -1

struct llhttp_settings_s:
llhttp_cb on_message_begin
Expand Down
24 changes: 14 additions & 10 deletions aiohttp/_http_parser.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,9 @@ cdef class HttpParser:

if self._more_data_available:
result = cb_on_body(self._cparser, b"", 0)
if result == -1:
# Exception already delivered to the payload in cb_on_body.
return EMPTY_FEED_DATA_RESULT
if result is cparser.HPE_PAUSED:
self._tail = data
return EMPTY_FEED_DATA_RESULT
Expand Down Expand Up @@ -799,7 +802,7 @@ cdef class HttpResponseParser(HttpParser):
else:
self._reason = self._reason or ''

cdef int cb_on_message_begin(cparser.llhttp_t* parser) except -1:
cdef int cb_on_message_begin(cparser.llhttp_t* parser) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data

pyparser._started = True
Expand All @@ -813,7 +816,7 @@ cdef int cb_on_message_begin(cparser.llhttp_t* parser) except -1:


cdef int cb_on_url(cparser.llhttp_t* parser,
const char *at, size_t length) except -1:
const char *at, size_t length) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
try:
if len(pyparser._buf) + length > pyparser._max_line_size:
Expand All @@ -828,7 +831,7 @@ cdef int cb_on_url(cparser.llhttp_t* parser,


cdef int cb_on_status(cparser.llhttp_t* parser,
const char *at, size_t length) except -1:
const char *at, size_t length) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
try:
if len(pyparser._buf) + length > pyparser._max_line_size:
Expand All @@ -843,7 +846,7 @@ cdef int cb_on_status(cparser.llhttp_t* parser,


cdef int cb_on_header_field(cparser.llhttp_t* parser,
const char *at, size_t length) except -1:
const char *at, size_t length) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
cdef Py_ssize_t size
try:
Expand All @@ -862,7 +865,7 @@ cdef int cb_on_header_field(cparser.llhttp_t* parser,


cdef int cb_on_header_value(cparser.llhttp_t* parser,
const char *at, size_t length) except -1:
const char *at, size_t length) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
cdef Py_ssize_t size
try:
Expand All @@ -878,7 +881,7 @@ cdef int cb_on_header_value(cparser.llhttp_t* parser,
return 0


cdef int cb_on_headers_complete(cparser.llhttp_t* parser) except -1:
cdef int cb_on_headers_complete(cparser.llhttp_t* parser) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
try:
pyparser._on_status_complete()
Expand All @@ -893,7 +896,7 @@ cdef int cb_on_headers_complete(cparser.llhttp_t* parser) except -1:


cdef int cb_on_body(cparser.llhttp_t* parser,
const char *at, size_t length) except -1:
const char *at, size_t length) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
cdef bytes body = at[:length]
while body or pyparser._more_data_available:
Expand All @@ -908,6 +911,7 @@ cdef int cb_on_body(cparser.llhttp_t* parser,

pyparser._payload_error = 1
pyparser._paused = False
pyparser._more_data_available = False
return -1
body = b""

Expand All @@ -918,7 +922,7 @@ cdef int cb_on_body(cparser.llhttp_t* parser,
return 0


cdef int cb_on_message_complete(cparser.llhttp_t* parser) except -1:
cdef int cb_on_message_complete(cparser.llhttp_t* parser) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
try:
pyparser._started = False
Expand All @@ -936,7 +940,7 @@ cdef int cb_on_message_complete(cparser.llhttp_t* parser) except -1:
return 0


cdef int cb_on_chunk_header(cparser.llhttp_t* parser) except -1:
cdef int cb_on_chunk_header(cparser.llhttp_t* parser) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
try:
pyparser._on_chunk_header()
Expand All @@ -947,7 +951,7 @@ cdef int cb_on_chunk_header(cparser.llhttp_t* parser) except -1:
return 0


cdef int cb_on_chunk_complete(cparser.llhttp_t* parser) except -1:
cdef int cb_on_chunk_complete(cparser.llhttp_t* parser) except? -1:
cdef HttpParser pyparser = <HttpParser>parser.data
try:
pyparser._on_chunk_complete()
Expand Down
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ exceptiongroup==1.3.1
# pytest
execnet==2.1.2
# via pytest-xdist
filelock==3.31.2
filelock==3.32.0
# via
# python-discovery
# virtualenv
Expand Down
2 changes: 1 addition & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ exceptiongroup==1.3.1
# pytest
execnet==2.1.2
# via pytest-xdist
filelock==3.31.2
filelock==3.32.0
# via
# python-discovery
# virtualenv
Expand Down
2 changes: 1 addition & 1 deletion requirements/lint.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ exceptiongroup==1.3.1
# via
# aiofastnet
# pytest
filelock==3.31.2
filelock==3.32.0
# via
# python-discovery
# virtualenv
Expand Down
46 changes: 46 additions & 0 deletions tests/test_http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,52 @@ async def test_compressed_with_tail(response: HttpResponseParser) -> None:
assert result == b"ok"


async def test_decompress_error_while_draining_pending_data(
response: HttpResponseParser,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Decompressor failure on the pending-data drain must not crash.

Regression test for https://github.com/aio-libs/aiohttp/issues/13203
where the C parser segfaulted on Python 3.12+ when the decompressor
raised while draining pending decompressed data, as brotlicffi 1.2
does on its empty-input drain call.
"""

class _BrokenDecompressor:
def __init__(self, *args: object, **kwargs: object) -> None:
self._calls = 0

def decompress_sync(self, data: bytes, max_length: int = 0) -> bytes:
self._calls += 1
if self._calls > 1:
raise ValueError("decoder is broken")
# Must be large enough to exceed the high water mark so the
# parser pauses with pending data still available.
return b"x" * (1024 * 1024)

@property
def data_available(self) -> bool:
return True

monkeypatch.setattr("aiohttp.http_parser.ZLibDecompressor", _BrokenDecompressor)

headers = (
b"HTTP/1.1 200 OK\r\n"
b"Content-Length: 100\r\n"
b"Content-Encoding: deflate\r\n"
b"\r\n"
)
# First byte 0x78 keeps the zlib header sniff from swapping decompressors.
msgs, upgrade, tail = response.feed_data(headers + b"\x78\x9c" + b"a" * 8)
payload = msgs[0][-1]

# The next feed drains pending data first, which hits the broken decoder.
# The parser must not raise; the error is delivered via the payload.
response.feed_data(b"b" * 10)
assert isinstance(payload.exception(), http_exceptions.ContentEncodingError)


async def test_two_content_length_responses_in_one_call(
response: HttpResponseParser,
) -> None:
Expand Down
Loading