From 16a24f6943abef4242f4fed0b1ac0189466a43b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Fri, 12 Jun 2026 16:02:12 +0200 Subject: [PATCH] fix(python): raise on mid-stream body errors instead of silent EOF A mid-stream body error (connection reset, truncated chunked transfer) was mapped to StopIteration/StopAsyncIteration, which signals normal end of iteration. The `async for`/`for` loop ended silently and the caller processed a partial body believing it was complete - a silent data-integrity bug. Now the classified ImpitError is propagated as a real exception, and the consumed/closed flags are set consistently with the clean-EOF branch. Streamed body truncation surfaces in reqwest as a `Decode`-kinded error rather than `Body`, so the existing unexpected-EOF classification missed it and fell through to the catch-all HTTPError. Widen the guard to cover `is_decode()` so truncated streams are reported as RemoteProtocolError, matching httpx. --- impit-python/src/response.rs | 10 +++----- impit-python/test/async_client_test.py | 34 +++++++++++++++++++++++++- impit-python/test/basic_client_test.py | 33 +++++++++++++++++++++++++ impit/src/errors.rs | 4 ++- 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/impit-python/src/response.rs b/impit-python/src/response.rs index 807adf18..a5cb5a15 100644 --- a/impit-python/src/response.rs +++ b/impit-python/src/response.rs @@ -63,12 +63,11 @@ impl PyResponseBytesIterator { if let Some(parent) = &slf.parent_response { if let Ok(mut parent_ref) = parent.try_borrow_mut(py) { parent_ref.inner_state = InnerResponseState::StreamingClosed; + parent_ref.is_stream_consumed = true; parent_ref.is_closed = true; } } - Err(pyo3::exceptions::PyStopIteration::new_err(format!( - "Stream error: {e}" - ))) + Err(ImpitPyError(ImpitError::from(e, None)).into()) } None => { slf.content_returned = true; @@ -155,13 +154,12 @@ impl PyResponseAsyncBytesIterator { Python::attach(|py| { if let Ok(mut parent_ref) = parent.try_borrow_mut(py) { parent_ref.inner_state = InnerResponseState::StreamingClosed; + parent_ref.is_stream_consumed = true; parent_ref.is_closed = true; } }); } - Err(pyo3::exceptions::PyStopAsyncIteration::new_err(format!( - "Stream error: {e}" - ))) + Err(ImpitPyError(ImpitError::from(e, None)).into()) } None => { if let Some(parent) = parent_response { diff --git a/impit-python/test/async_client_test.py b/impit-python/test/async_client_test.py index 0e8f805c..8a0b11b5 100644 --- a/impit-python/test/async_client_test.py +++ b/impit-python/test/async_client_test.py @@ -7,7 +7,7 @@ import pytest -from impit import AsyncClient, Browser, Cookies, StreamClosed, StreamConsumed, TooManyRedirects +from impit import AsyncClient, Browser, Cookies, RemoteProtocolError, StreamClosed, StreamConsumed, TooManyRedirects from .httpbin import get_httpbin_url from .setup_proxy import start_proxy_server @@ -30,6 +30,22 @@ def thread_server(port_holder: list[int]) -> None: server.close() +def truncating_server(port_holder: list[int]) -> None: + """Announce a `Content-Length` larger than the body actually sent, then close the socket mid-body.""" + server = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) + server.bind(('::', 0)) + port_holder[0] = server.getsockname()[1] + server.listen(1) + + conn, _ = server.accept() + conn.recv(1024) + conn.send(b'HTTP/1.1 200 OK\r\nContent-Length: 100\r\n\r\n0123456789') + conn.close() + server.close() + + @pytest.mark.asyncio @pytest.mark.parametrize( ('browser', 'ja4'), @@ -638,3 +654,19 @@ async def test_iter_bytes_without_consumed(self, browser: Browser) -> None: with pytest.raises(StreamClosed): _ = response.content + + async def test_truncated_stream_raises(self, browser: Browser) -> None: + port_holder = [0] + thread = threading.Thread(target=truncating_server, args=(port_holder,)) + thread.start() + await asyncio.sleep(0.1) + + impit = AsyncClient(browser=browser) + + async with impit.stream('GET', f'http://localhost:{port_holder[0]}/', timeout=5) as response: + assert response.status_code == 200 + + with pytest.raises(RemoteProtocolError): + _ = b''.join([item async for item in response.aiter_bytes()]) + + thread.join() diff --git a/impit-python/test/basic_client_test.py b/impit-python/test/basic_client_test.py index 635738d0..7da84e07 100644 --- a/impit-python/test/basic_client_test.py +++ b/impit-python/test/basic_client_test.py @@ -14,6 +14,7 @@ ConnectTimeout, Cookies, ReadTimeout, + RemoteProtocolError, StreamClosed, StreamConsumed, TimeoutException, @@ -41,6 +42,22 @@ def thread_server(port_holder: list[int]) -> None: server.close() +def truncating_server(port_holder: list[int]) -> None: + """Announce a `Content-Length` larger than the body actually sent, then close the socket mid-body.""" + server = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) + server.bind(('::', 0)) + port_holder[0] = server.getsockname()[1] + server.listen(1) + + conn, _ = server.accept() + conn.recv(1024) + conn.send(b'HTTP/1.1 200 OK\r\nContent-Length: 100\r\n\r\n0123456789') + conn.close() + server.close() + + @pytest.mark.parametrize( ('browser', 'ja4'), [ @@ -611,6 +628,22 @@ def test_iter_bytes_without_consumed(self, browser: Browser) -> None: with pytest.raises(StreamClosed): _ = response.content + def test_truncated_stream_raises(self, browser: Browser) -> None: + port_holder = [0] + thread = threading.Thread(target=truncating_server, args=(port_holder,)) + thread.start() + time.sleep(0.1) + + impit = Client(browser=browser) + + with impit.stream('GET', f'http://localhost:{port_holder[0]}/', timeout=5) as response: + assert response.status_code == 200 + + with pytest.raises(RemoteProtocolError): + _ = b''.join(response.iter_bytes()) + + thread.join() + def make_slow_server(port_holder: list[int], delay: float = 2.0) -> None: """Start a server in a daemon thread that waits `delay` seconds before responding.""" diff --git a/impit/src/errors.rs b/impit/src/errors.rs index 8bcbf959..d815590b 100644 --- a/impit/src/errors.rs +++ b/impit/src/errors.rs @@ -122,7 +122,9 @@ impl ImpitError { return ImpitError::TooManyRedirects(context.max_redirects); } - if error.is_body() && (format!("{:?}", error).to_lowercase()).contains("unexpectedeof") { + if (error.is_body() || error.is_decode()) + && (format!("{:?}", error).to_lowercase()).contains("unexpectedeof") + { return ImpitError::RemoteProtocolError; }