From 09185402fecae8a068ce391a690dd1fc60a45bcf Mon Sep 17 00:00:00 2001 From: Zanie Adkins Date: Fri, 5 May 2023 16:44:35 -0500 Subject: [PATCH 1/6] Add check for `h2.connection.ConnectionState.CLOSED` in `AsyncHTTP2Connection.is_available` --- httpcore/_async/http2.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/httpcore/_async/http2.py b/httpcore/_async/http2.py index 035cbcd3..fa8062ab 100644 --- a/httpcore/_async/http2.py +++ b/httpcore/_async/http2.py @@ -433,6 +433,10 @@ def is_available(self) -> bool: self._state != HTTPConnectionState.CLOSED and not self._connection_error and not self._used_all_stream_ids + and not ( + self._h2_state.state_machine.state + == h2.connection.ConnectionState.CLOSED + ) ) def has_expired(self) -> bool: From 08e717f1b440c1f30420119ae08f519544d773c6 Mon Sep 17 00:00:00 2001 From: Zanie Adkins Date: Fri, 5 May 2023 17:09:30 -0500 Subject: [PATCH 2/6] Add sync implementation --- httpcore/_sync/http2.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/httpcore/_sync/http2.py b/httpcore/_sync/http2.py index 35426be5..8e2f55e0 100644 --- a/httpcore/_sync/http2.py +++ b/httpcore/_sync/http2.py @@ -433,6 +433,10 @@ def is_available(self) -> bool: self._state != HTTPConnectionState.CLOSED and not self._connection_error and not self._used_all_stream_ids + and not ( + self._h2_state.state_machine.state + == h2.connection.ConnectionState.CLOSED + ) ) def has_expired(self) -> bool: From 00f98c55493b1c172d748e32d9a4e566362bb9b9 Mon Sep 17 00:00:00 2001 From: Zanie Adkins Date: Wed, 10 May 2023 10:22:17 -0500 Subject: [PATCH 3/6] Add test for closed connection --- tests/_async/test_http2.py | 33 ++++++++++++++++++++++++++++++ tests/_sync/test_http2.py | 42 ++++++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/tests/_async/test_http2.py b/tests/_async/test_http2.py index c719dbed..2356a5b7 100644 --- a/tests/_async/test_http2.py +++ b/tests/_async/test_http2.py @@ -52,6 +52,39 @@ async def test_http2_connection(): ) +async def test_http2_connection_closed(): + origin = Origin(b"https", b"example.com", 443) + stream = AsyncMockStream( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.HeadersFrame( + stream_id=1, + data=hpack.Encoder().encode( + [ + (b":status", b"200"), + (b"content-type", b"plain/text"), + ] + ), + flags=["END_HEADERS"], + ).serialize(), + hyperframe.frame.DataFrame( + stream_id=1, data=b"Hello, world!", flags=["END_STREAM"] + ).serialize(), + # Connection is closed after the first response + hyperframe.frame.GoAwayFrame(stream_id=0, error_code=0).serialize(), + ] + ) + with AsyncHTTP2Connection( + origin=origin, stream=stream, keepalive_expiry=5.0 + ) as conn: + await conn.request("GET", "https://example.com/") + + with pytest.raises(RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + assert not conn.is_available() + + @pytest.mark.anyio async def test_http2_connection_post_request(): origin = Origin(b"https", b"example.com", 443) diff --git a/tests/_sync/test_http2.py b/tests/_sync/test_http2.py index e17dfa9e..563e4a86 100644 --- a/tests/_sync/test_http2.py +++ b/tests/_sync/test_http2.py @@ -11,7 +11,6 @@ from httpcore.backends.mock import MockStream - def test_http2_connection(): origin = Origin(b"https", b"example.com", 443) stream = MockStream( @@ -32,9 +31,7 @@ def test_http2_connection(): ).serialize(), ] ) - with HTTP2Connection( - origin=origin, stream=stream, keepalive_expiry=5.0 - ) as conn: + with HTTP2Connection(origin=origin, stream=stream, keepalive_expiry=5.0) as conn: response = conn.request("GET", "https://example.com/") assert response.status == 200 assert response.content == b"Hello, world!" @@ -52,6 +49,36 @@ def test_http2_connection(): ) +def test_http2_connection_closed(): + origin = Origin(b"https", b"example.com", 443) + stream = MockStream( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.HeadersFrame( + stream_id=1, + data=hpack.Encoder().encode( + [ + (b":status", b"200"), + (b"content-type", b"plain/text"), + ] + ), + flags=["END_HEADERS"], + ).serialize(), + hyperframe.frame.DataFrame( + stream_id=1, data=b"Hello, world!", flags=["END_STREAM"] + ).serialize(), + # Connection is closed after the first response + hyperframe.frame.GoAwayFrame(stream_id=0, error_code=0).serialize(), + ] + ) + with HTTP2Connection(origin=origin, stream=stream, keepalive_expiry=5.0) as conn: + conn.request("GET", "https://example.com/") + + with pytest.raises(RemoteProtocolError): + conn.request("GET", "https://example.com/") + + assert not conn.is_available() + def test_http2_connection_post_request(): origin = Origin(b"https", b"example.com", 443) @@ -84,7 +111,6 @@ def test_http2_connection_post_request(): assert response.content == b"Hello, world!" - def test_http2_connection_with_remote_protocol_error(): """ If a remote protocol error occurs, then no response will be returned, @@ -97,7 +123,6 @@ def test_http2_connection_with_remote_protocol_error(): conn.request("GET", "https://example.com/") - def test_http2_connection_with_rst_stream(): """ If a stream reset occurs, then no response will be returned, @@ -143,7 +168,6 @@ def test_http2_connection_with_rst_stream(): assert response.status == 200 - def test_http2_connection_with_goaway(): """ If a stream reset occurs, then no response will be returned, @@ -189,7 +213,6 @@ def test_http2_connection_with_goaway(): conn.request("GET", "https://example.com/") - def test_http2_connection_with_flow_control(): origin = Origin(b"https", b"example.com", 443) stream = MockStream( @@ -249,7 +272,6 @@ def test_http2_connection_with_flow_control(): assert response.content == b"100,000 bytes received" - def test_http2_connection_attempt_close(): """ A connection can only be closed when it is idle. @@ -284,7 +306,6 @@ def test_http2_connection_attempt_close(): conn.request("GET", "https://example.com/") - def test_http2_request_to_incorrect_origin(): """ A connection can only send requests to whichever origin it is connected to. @@ -296,7 +317,6 @@ def test_http2_request_to_incorrect_origin(): conn.request("GET", "https://other.com/") - def test_http2_remote_max_streams_update(): """ If the remote server updates the maximum concurrent streams value, we should From f0d3acd02288f8b47252452ed41b9bcad1056674 Mon Sep 17 00:00:00 2001 From: Zanie Adkins Date: Wed, 10 May 2023 10:24:42 -0500 Subject: [PATCH 4/6] Regenerate sync tests with `unasync` --- tests/_sync/test_http2.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/_sync/test_http2.py b/tests/_sync/test_http2.py index 563e4a86..1cf90feb 100644 --- a/tests/_sync/test_http2.py +++ b/tests/_sync/test_http2.py @@ -11,6 +11,7 @@ from httpcore.backends.mock import MockStream + def test_http2_connection(): origin = Origin(b"https", b"example.com", 443) stream = MockStream( @@ -31,7 +32,9 @@ def test_http2_connection(): ).serialize(), ] ) - with HTTP2Connection(origin=origin, stream=stream, keepalive_expiry=5.0) as conn: + with HTTP2Connection( + origin=origin, stream=stream, keepalive_expiry=5.0 + ) as conn: response = conn.request("GET", "https://example.com/") assert response.status == 200 assert response.content == b"Hello, world!" @@ -71,7 +74,9 @@ def test_http2_connection_closed(): hyperframe.frame.GoAwayFrame(stream_id=0, error_code=0).serialize(), ] ) - with HTTP2Connection(origin=origin, stream=stream, keepalive_expiry=5.0) as conn: + with HTTP2Connection( + origin=origin, stream=stream, keepalive_expiry=5.0 + ) as conn: conn.request("GET", "https://example.com/") with pytest.raises(RemoteProtocolError): @@ -80,6 +85,7 @@ def test_http2_connection_closed(): assert not conn.is_available() + def test_http2_connection_post_request(): origin = Origin(b"https", b"example.com", 443) stream = MockStream( @@ -111,6 +117,7 @@ def test_http2_connection_post_request(): assert response.content == b"Hello, world!" + def test_http2_connection_with_remote_protocol_error(): """ If a remote protocol error occurs, then no response will be returned, @@ -123,6 +130,7 @@ def test_http2_connection_with_remote_protocol_error(): conn.request("GET", "https://example.com/") + def test_http2_connection_with_rst_stream(): """ If a stream reset occurs, then no response will be returned, @@ -168,6 +176,7 @@ def test_http2_connection_with_rst_stream(): assert response.status == 200 + def test_http2_connection_with_goaway(): """ If a stream reset occurs, then no response will be returned, @@ -213,6 +222,7 @@ def test_http2_connection_with_goaway(): conn.request("GET", "https://example.com/") + def test_http2_connection_with_flow_control(): origin = Origin(b"https", b"example.com", 443) stream = MockStream( @@ -272,6 +282,7 @@ def test_http2_connection_with_flow_control(): assert response.content == b"100,000 bytes received" + def test_http2_connection_attempt_close(): """ A connection can only be closed when it is idle. @@ -306,6 +317,7 @@ def test_http2_connection_attempt_close(): conn.request("GET", "https://example.com/") + def test_http2_request_to_incorrect_origin(): """ A connection can only send requests to whichever origin it is connected to. @@ -317,6 +329,7 @@ def test_http2_request_to_incorrect_origin(): conn.request("GET", "https://other.com/") + def test_http2_remote_max_streams_update(): """ If the remote server updates the maximum concurrent streams value, we should From 9769dcaabc27f4e1897346a912840ac2e4de8209 Mon Sep 17 00:00:00 2001 From: Zanie Adkins Date: Wed, 10 May 2023 10:32:26 -0500 Subject: [PATCH 5/6] Use async with --- tests/_async/test_http2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/_async/test_http2.py b/tests/_async/test_http2.py index 2356a5b7..2c415e42 100644 --- a/tests/_async/test_http2.py +++ b/tests/_async/test_http2.py @@ -74,7 +74,7 @@ async def test_http2_connection_closed(): hyperframe.frame.GoAwayFrame(stream_id=0, error_code=0).serialize(), ] ) - with AsyncHTTP2Connection( + async with AsyncHTTP2Connection( origin=origin, stream=stream, keepalive_expiry=5.0 ) as conn: await conn.request("GET", "https://example.com/") From ad04f9c6210d564b5ee967a962ba42b71177f78c Mon Sep 17 00:00:00 2001 From: Zanie Adkins Date: Wed, 10 May 2023 12:33:03 -0500 Subject: [PATCH 6/6] Add anyio annotation --- tests/_async/test_http2.py | 1 + tests/_sync/test_http2.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/_async/test_http2.py b/tests/_async/test_http2.py index 2c415e42..bd27df56 100644 --- a/tests/_async/test_http2.py +++ b/tests/_async/test_http2.py @@ -52,6 +52,7 @@ async def test_http2_connection(): ) +@pytest.mark.anyio async def test_http2_connection_closed(): origin = Origin(b"https", b"example.com", 443) stream = AsyncMockStream( diff --git a/tests/_sync/test_http2.py b/tests/_sync/test_http2.py index 1cf90feb..df4cfcbc 100644 --- a/tests/_sync/test_http2.py +++ b/tests/_sync/test_http2.py @@ -52,6 +52,7 @@ def test_http2_connection(): ) + def test_http2_connection_closed(): origin = Origin(b"https", b"example.com", 443) stream = MockStream(