diff --git a/CHANGES/13203.bugfix.rst b/CHANGES/13203.bugfix.rst new file mode 100644 index 00000000000..5b53eeaceba --- /dev/null +++ b/CHANGES/13203.bugfix.rst @@ -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`. diff --git a/CHANGES/13249.bugfix.rst b/CHANGES/13249.bugfix.rst new file mode 120000 index 00000000000..11b0a503dbc --- /dev/null +++ b/CHANGES/13249.bugfix.rst @@ -0,0 +1 @@ +13203.bugfix.rst \ No newline at end of file diff --git a/aiohttp/_cparser.pxd b/aiohttp/_cparser.pxd index 903f35c4590..09b60a1053a 100644 --- a/aiohttp/_cparser.pxd +++ b/aiohttp/_cparser.pxd @@ -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 diff --git a/aiohttp/_http_parser.pyx b/aiohttp/_http_parser.pyx index 3808417b0cf..720115e652a 100644 --- a/aiohttp/_http_parser.pyx +++ b/aiohttp/_http_parser.pyx @@ -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 @@ -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 = parser.data pyparser._started = True @@ -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 = parser.data try: if len(pyparser._buf) + length > pyparser._max_line_size: @@ -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 = parser.data try: if len(pyparser._buf) + length > pyparser._max_line_size: @@ -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 = parser.data cdef Py_ssize_t size try: @@ -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 = parser.data cdef Py_ssize_t size try: @@ -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 = parser.data try: pyparser._on_status_complete() @@ -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 = parser.data cdef bytes body = at[:length] while body or pyparser._more_data_available: @@ -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"" @@ -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 = parser.data try: pyparser._started = False @@ -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 = parser.data try: pyparser._on_chunk_header() @@ -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 = parser.data try: pyparser._on_chunk_complete() diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 3138dfc93b3..d9868ae93ce 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -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 diff --git a/requirements/dev.txt b/requirements/dev.txt index f2c759ec33f..ffa43428bf3 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -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 diff --git a/requirements/lint.txt b/requirements/lint.txt index 3c938421824..778b9b7d5e5 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -42,7 +42,7 @@ exceptiongroup==1.3.1 # via # aiofastnet # pytest -filelock==3.31.2 +filelock==3.32.0 # via # python-discovery # virtualenv diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index f080cd208ff..a3a4ee9db13 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -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: