diff --git a/CHANGELOG.md b/CHANGELOG.md index d44ce896..c4517547 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased +- Handle HTTP/2 half-closed connections gracefully. (#777) - Add support for Python 3.12. (#807) ## 0.18.0 (September 8th, 2023) diff --git a/httpcore/_async/http2.py b/httpcore/_async/http2.py index 8dc776ff..45565da2 100644 --- a/httpcore/_async/http2.py +++ b/httpcore/_async/http2.py @@ -138,10 +138,16 @@ async def handle_async_request(self, request: Request) -> Response: try: kwargs = {"request": request, "stream_id": stream_id} - async with Trace("send_request_headers", logger, request, kwargs): - await self._send_request_headers(request=request, stream_id=stream_id) - async with Trace("send_request_body", logger, request, kwargs): - await self._send_request_body(request=request, stream_id=stream_id) + try: + async with Trace("send_request_headers", logger, request, kwargs): + await self._send_request_headers( + request=request, stream_id=stream_id + ) + async with Trace("send_request_body", logger, request, kwargs): + await self._send_request_body(request=request, stream_id=stream_id) + except h2.exceptions.StreamClosedError: + pass + async with Trace( "receive_response_headers", logger, request, kwargs ) as trace: diff --git a/httpcore/_sync/http2.py b/httpcore/_sync/http2.py index d141d459..33ce661d 100644 --- a/httpcore/_sync/http2.py +++ b/httpcore/_sync/http2.py @@ -138,10 +138,16 @@ def handle_request(self, request: Request) -> Response: try: kwargs = {"request": request, "stream_id": stream_id} - with Trace("send_request_headers", logger, request, kwargs): - self._send_request_headers(request=request, stream_id=stream_id) - with Trace("send_request_body", logger, request, kwargs): - self._send_request_body(request=request, stream_id=stream_id) + try: + with Trace("send_request_headers", logger, request, kwargs): + self._send_request_headers( + request=request, stream_id=stream_id + ) + with Trace("send_request_body", logger, request, kwargs): + self._send_request_body(request=request, stream_id=stream_id) + except h2.exceptions.StreamClosedError: + pass + with Trace( "receive_response_headers", logger, request, kwargs ) as trace: diff --git a/tests/_async/test_connection.py b/tests/_async/test_connection.py index b6ee0c7e..e68c56d6 100644 --- a/tests/_async/test_connection.py +++ b/tests/_async/test_connection.py @@ -141,6 +141,56 @@ async def connect_tcp( assert response.content == b"Request body exceeded 1,000,000 bytes" +@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") +@pytest.mark.anyio +async def test_write_error_with_response_sent_http2(): + """ + If a server half-closes the connection while the client is sending + the request, it may still send a response. In this case the client + should successfully read and return the response. + + See also the `test_write_error_without_response_sent_http2` test above. + """ + + origin = Origin(b"https", b"example.com", 443) + network_backend = AsyncMockBackend( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.WindowUpdateFrame( + stream_id=0, window_increment=2147418112 + ).serialize() + + hyperframe.frame.WindowUpdateFrame( + stream_id=1, window_increment=2147418112 + ).serialize() + + hyperframe.frame.HeadersFrame( + stream_id=1, + data=hpack.Encoder().encode( + [ + (b":status", b"413"), + (b"content-type", b"plain/text"), + ] + ), + flags=["END_HEADERS"], + ).serialize() + + hyperframe.frame.DataFrame( + stream_id=1, + data=b"Request body exceeded 1,000,000 bytes", + flags=["END_STREAM"], + ).serialize() + + hyperframe.frame.RstStreamFrame(stream_id=1, error_code=0).serialize(), + ], + http2=True, + ) + + async with AsyncHTTPConnection( + origin=origin, network_backend=network_backend, keepalive_expiry=5.0 + ) as conn: + content = b"x" * 10_000_000 + response = await conn.request("POST", "https://example.com/", content=content) + assert response.status == 413 + assert response.content == b"Request body exceeded 1,000,000 bytes" + + @pytest.mark.anyio @pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") async def test_write_error_without_response_sent(): @@ -184,9 +234,47 @@ async def connect_tcp( content = b"x" * 10_000_000 with pytest.raises(RemoteProtocolError) as exc_info: await conn.request("POST", "https://example.com/", content=content) + assert str(exc_info.value) == "Server disconnected without sending a response." +@pytest.mark.anyio +@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") +async def test_write_error_without_response_sent_http2(): + """ + If a server fully closes the connection while the client is sending + the request, then client should raise an error. + + See also the `test_write_error_with_response_sent_http2` test above. + """ + + origin = Origin(b"https", b"example.com", 443) + network_backend = AsyncMockBackend( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.WindowUpdateFrame( + stream_id=0, window_increment=2147418112 + ).serialize() + + hyperframe.frame.WindowUpdateFrame( + stream_id=1, window_increment=2147418112 + ).serialize() + + hyperframe.frame.RstStreamFrame(stream_id=1, error_code=0).serialize(), + ], + http2=True, + ) + + async with AsyncHTTPConnection( + origin=origin, network_backend=network_backend, keepalive_expiry=5.0 + ) as conn: + content = b"x" * 10_000_000 + with pytest.raises(RemoteProtocolError) as exc_info: + await conn.request("POST", "https://example.com/", content=content) + assert str(exc_info.value) in [ + "", + "", + ] + + @pytest.mark.anyio @pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") async def test_http2_connection(): diff --git a/tests/_sync/test_connection.py b/tests/_sync/test_connection.py index 37c82e02..757a309f 100644 --- a/tests/_sync/test_connection.py +++ b/tests/_sync/test_connection.py @@ -141,6 +141,56 @@ def connect_tcp( assert response.content == b"Request body exceeded 1,000,000 bytes" +@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") + +def test_write_error_with_response_sent_http2(): + """ + If a server half-closes the connection while the client is sending + the request, it may still send a response. In this case the client + should successfully read and return the response. + + See also the `test_write_error_without_response_sent_http2` test above. + """ + + origin = Origin(b"https", b"example.com", 443) + network_backend = MockBackend( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.WindowUpdateFrame( + stream_id=0, window_increment=2147418112 + ).serialize() + + hyperframe.frame.WindowUpdateFrame( + stream_id=1, window_increment=2147418112 + ).serialize() + + hyperframe.frame.HeadersFrame( + stream_id=1, + data=hpack.Encoder().encode( + [ + (b":status", b"413"), + (b"content-type", b"plain/text"), + ] + ), + flags=["END_HEADERS"], + ).serialize() + + hyperframe.frame.DataFrame( + stream_id=1, + data=b"Request body exceeded 1,000,000 bytes", + flags=["END_STREAM"], + ).serialize() + + hyperframe.frame.RstStreamFrame(stream_id=1, error_code=0).serialize(), + ], + http2=True, + ) + + with HTTPConnection( + origin=origin, network_backend=network_backend, keepalive_expiry=5.0 + ) as conn: + content = b"x" * 10_000_000 + response = conn.request("POST", "https://example.com/", content=content) + assert response.status == 413 + assert response.content == b"Request body exceeded 1,000,000 bytes" + + @pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") def test_write_error_without_response_sent(): @@ -184,10 +234,48 @@ def connect_tcp( content = b"x" * 10_000_000 with pytest.raises(RemoteProtocolError) as exc_info: conn.request("POST", "https://example.com/", content=content) + assert str(exc_info.value) == "Server disconnected without sending a response." +@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") +def test_write_error_without_response_sent_http2(): + """ + If a server fully closes the connection while the client is sending + the request, then client should raise an error. + + See also the `test_write_error_with_response_sent_http2` test above. + """ + + origin = Origin(b"https", b"example.com", 443) + network_backend = MockBackend( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.WindowUpdateFrame( + stream_id=0, window_increment=2147418112 + ).serialize() + + hyperframe.frame.WindowUpdateFrame( + stream_id=1, window_increment=2147418112 + ).serialize() + + hyperframe.frame.RstStreamFrame(stream_id=1, error_code=0).serialize(), + ], + http2=True, + ) + + with HTTPConnection( + origin=origin, network_backend=network_backend, keepalive_expiry=5.0 + ) as conn: + content = b"x" * 10_000_000 + with pytest.raises(RemoteProtocolError) as exc_info: + conn.request("POST", "https://example.com/", content=content) + assert str(exc_info.value) in [ + "", + "", + ] + + + @pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning") def test_http2_connection(): origin = Origin(b"https", b"example.com", 443)