From 4afd59c27bf438a54ed4c856b51d8a6dfe231c86 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 19 Sep 2016 12:29:18 +0100 Subject: [PATCH 1/4] Receiving a PUSH_PROMISE from a client is an error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is explicitly forbidden in the spec (RFC 7540 § 8.2). In practice hyper-h2 was already raising a ProtocolError, because local_settings.enable_push is False for servers. This commit: * Tweaks the error message to make it clear that receiving it from the client is important. * Add an explicit test case. --- h2/connection.py | 4 ++++ test/test_basic_logic.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/h2/connection.py b/h2/connection.py index e431db312..a154f726a 100644 --- a/h2/connection.py +++ b/h2/connection.py @@ -1487,6 +1487,10 @@ def _receive_push_promise_frame(self, frame): """ Receive a push-promise frame on the connection. """ + # A client cannot push. - RFC 7540 § 8.2 + if not self.config.client_side: + raise ProtocolError("Received pushed stream from a client") + if not self.local_settings.enable_push: raise ProtocolError("Received pushed stream") diff --git a/test/test_basic_logic.py b/test/test_basic_logic.py index 1664c2408..c96b16569 100644 --- a/test/test_basic_logic.py +++ b/test/test_basic_logic.py @@ -1523,3 +1523,32 @@ def test_receiving_goaway_frame_with_unknown_error(self, frame_factory): assert c.state_machine.state == h2.connection.ConnectionState.CLOSED assert not c.data_to_send() + + def test_receiving_push_promise_from_client_is_error(self, frame_factory): + """ + If we are a server, receiving PUSH_PROMISE frames from a client + causes the connection to be closed. + """ + c = h2.connection.H2Connection(client_side=False) + c.initiate_connection() + c.receive_data(frame_factory.preamble()) + + f1 = frame_factory.build_headers_frame( + self.example_request_headers + ) + f2 = frame_factory.build_push_promise_frame( + stream_id=1, + promised_stream_id=2, + headers=self.example_request_headers, + flags=['END_HEADERS'], + ) + c.receive_data(f1.serialize()) + c.clear_outbound_data_buffer() + + with pytest.raises(h2.exceptions.ProtocolError): + c.receive_data(f2.serialize()) + + expected_frame = frame_factory.build_goaway_frame( + 1, h2.errors.ErrorCodes.PROTOCOL_ERROR + ) + assert c.data_to_send() == expected_frame.serialize() From 797772fcb056a41e3d1464a4fde3d870e4dc002a Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 19 Sep 2016 12:32:40 +0100 Subject: [PATCH 2/4] That full stop annoys me --- h2/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/h2/connection.py b/h2/connection.py index a154f726a..3c2940f41 100644 --- a/h2/connection.py +++ b/h2/connection.py @@ -1487,7 +1487,7 @@ def _receive_push_promise_frame(self, frame): """ Receive a push-promise frame on the connection. """ - # A client cannot push. - RFC 7540 § 8.2 + # A client cannot push - RFC 7540 § 8.2 if not self.config.client_side: raise ProtocolError("Received pushed stream from a client") From 934d013eb13f5231910cd1c63cfbfdddce3dd11e Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 19 Sep 2016 19:41:17 +0100 Subject: [PATCH 3/4] Make sure the test catches the correct exception message --- test/test_basic_logic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_basic_logic.py b/test/test_basic_logic.py index c96b16569..5d3f41d4b 100644 --- a/test/test_basic_logic.py +++ b/test/test_basic_logic.py @@ -1545,8 +1545,9 @@ def test_receiving_push_promise_from_client_is_error(self, frame_factory): c.receive_data(f1.serialize()) c.clear_outbound_data_buffer() - with pytest.raises(h2.exceptions.ProtocolError): + with pytest.raises(h2.exceptions.ProtocolError) as excinfo: c.receive_data(f2.serialize()) + excinfo.match('Received pushed stream from a client') expected_frame = frame_factory.build_goaway_frame( 1, h2.errors.ErrorCodes.PROTOCOL_ERROR From 3bd24b8ff7a8326d27486687f1656539781485bb Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 19 Sep 2016 19:57:40 +0100 Subject: [PATCH 4/4] Add a changelog entry --- HISTORY.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 62a533cd5..903d918c9 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -56,6 +56,8 @@ Bugfixes 7540 Section 8.1.2.1. - Correctly refuse to send trailers that contain HTTP/2 pseudo-header fields, per RFC 7540 Section 8.1.2.1. +- Correctly reject a PUSH_PROMISE frame sent by a client, per RFC 7540 + Section 8.2. 2.4.0 (2016-07-01)