From 0a827f70a094a3b4b308db61adff9d8354bfd726 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Fri, 24 Mar 2017 08:37:00 +0000 Subject: [PATCH 1/3] Change the default header encoding to None --- h2/config.py | 22 +++++++++++++--------- h2/connection.py | 1 - 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/h2/config.py b/h2/config.py index d3e411fde..08129a406 100644 --- a/h2/config.py +++ b/h2/config.py @@ -62,10 +62,14 @@ class H2Configuration(object): :param header_encoding: Controls whether the headers emitted by this object in events are transparently decoded to ``unicode`` strings, and what - encoding is used to do that decoding. For historical reasons, this - defaults to ``'utf-8'``. To prevent the decoding of headers (that is, - to force them to be returned as bytestrings), this can be set to - ``False`` or the empty string. + encoding is used to do that decoding. This defaults to ``None``, + meaning that headers will be returned as bytes. To automatically + decode headers (that is, to return them as unicode strings), this can + be set to the string name of any encoding, e.g. ``'utf-8'``. + + .. versionchanged:: 3.0.0 + Changed default value from ``'utf-8'`` to ``None`` + :type header_encoding: ``str``, ``False``, or ``None`` :param validate_outbound_headers: Controls whether the headers emitted @@ -122,7 +126,7 @@ class H2Configuration(object): def __init__(self, client_side=True, - header_encoding='utf-8', + header_encoding=None, validate_outbound_headers=True, normalize_outbound_headers=True, validate_inbound_headers=True, @@ -141,10 +145,10 @@ def header_encoding(self): """ Controls whether the headers emitted by this object in events are transparently decoded to ``unicode`` strings, and what encoding is used - to do that decoding. For historical reasons, this defaults to - ``'utf-8'``. To prevent the decoding of headers (that is, to force them - to be returned as bytestrings), this can be set to ``False`` or the - empty string. + to do that decoding. This defaults to ``None``, meaning that headers + will be returned as bytes. To automatically decode headers (that is, to + return them as unicode strings), this can be set to the string name of + any encoding, e.g. ``'utf-8'``. """ return self._header_encoding diff --git a/h2/connection.py b/h2/connection.py index 5cdf6a0c4..1e77c4a25 100644 --- a/h2/connection.py +++ b/h2/connection.py @@ -300,7 +300,6 @@ def __init__(self, config=None): if self.config is None: self.config = H2Configuration( client_side=True, - header_encoding='utf-8', ) # Objects that store settings, including defaults. From 75044d1de67795a4afb417978e97b0437dce0bd8 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Fri, 24 Mar 2017 09:26:21 +0000 Subject: [PATCH 2/3] Fixup tests relying on old behaviour. This change kinda semi-randomly swaps some tests to use the new default behaviour, and has some opt-in to the old behaviour, to ensure that both continue to work --- test/test_basic_logic.py | 24 +++++++++++++----- test/test_complex_logic.py | 32 +++++++++++------------ test/test_config.py | 2 +- test/test_h2_upgrade.py | 24 +++++++++--------- test/test_head_request.py | 14 +++++----- test/test_informational_responses.py | 38 ++++++++++++++-------------- test/test_interacting_stacks.py | 16 ++++++------ test/test_invalid_headers.py | 11 +++++--- test/test_stream_reset.py | 14 +++++----- 9 files changed, 95 insertions(+), 80 deletions(-) diff --git a/test/test_basic_logic.py b/test/test_basic_logic.py index d4df95076..1144d40db 100644 --- a/test/test_basic_logic.py +++ b/test/test_basic_logic.py @@ -232,7 +232,8 @@ def test_receiving_a_response(self, frame_factory): """ When receiving a response, the ResponseReceived event fires. """ - c = h2.connection.H2Connection() + config = h2.config.H2Configuration(header_encoding='utf-8') + c = h2.connection.H2Connection(config=config) c.initiate_connection() c.send_headers(1, self.example_request_headers, end_stream=True) @@ -346,7 +347,8 @@ def test_receiving_pushed_stream(self, frame_factory): Pushed streams fire a PushedStreamReceived event, followed by ResponseReceived when the response headers are received. """ - c = h2.connection.H2Connection() + config = h2.config.H2Configuration(header_encoding='utf-8') + c = h2.connection.H2Connection(config=config) c.initiate_connection() c.send_headers(1, self.example_request_headers, end_stream=False) @@ -660,7 +662,8 @@ def test_can_receive_trailers(self, frame_factory): When two HEADERS blocks are received in the same stream from a server, the second set are trailers. """ - c = h2.connection.H2Connection() + config = h2.config.H2Configuration(header_encoding='utf-8') + c = h2.connection.H2Connection(config=config) c.initiate_connection() c.send_headers(1, self.example_request_headers) f = frame_factory.build_headers_frame(self.example_response_headers) @@ -842,7 +845,8 @@ def test_cookies_are_joined_on_push(self, frame_factory): 'path=1; test1=val1; test2=val2' ) - c = h2.connection.H2Connection() + config = h2.config.H2Configuration(header_encoding='utf-8') + c = h2.connection.H2Connection(config=config) c.initiate_connection() c.send_headers(1, self.example_request_headers, end_stream=True) @@ -877,7 +881,9 @@ def test_cookies_arent_joined_without_normalization(self, frame_factory): ] config = h2.config.H2Configuration( - client_side=True, normalize_inbound_headers=False + client_side=True, + normalize_inbound_headers=False, + header_encoding='utf-8' ) c = h2.connection.H2Connection(config=config) c.initiate_connection() @@ -918,7 +924,9 @@ class TestBasicServer(object): (':status', '200'), ('server', 'hyper-h2/0.1.0') ] - server_config = h2.config.H2Configuration(client_side=False) + server_config = h2.config.H2Configuration( + client_side=False, header_encoding='utf-8' + ) def test_ignores_preamble(self): """ @@ -1787,7 +1795,9 @@ def test_cookies_arent_joined_without_normalization(self, frame_factory): ] config = h2.config.H2Configuration( - client_side=False, normalize_inbound_headers=False + client_side=False, + normalize_inbound_headers=False, + header_encoding='utf-8' ) c = h2.connection.H2Connection(config=config) c.initiate_connection() diff --git a/test/test_complex_logic.py b/test/test_complex_logic.py index 1ca64fa93..ff90bb8bf 100644 --- a/test/test_complex_logic.py +++ b/test/test_complex_logic.py @@ -110,14 +110,14 @@ class TestComplexServer(object): Complex tests for server-side stacks. """ example_request_headers = [ - (':authority', 'example.com'), - (':path', '/'), - (':scheme', 'https'), - (':method', 'GET'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), ] example_response_headers = [ - (':status', '200'), - ('server', 'fake-serv/0.1.0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0') ] server_config = h2.config.H2Configuration(client_side=False) @@ -194,10 +194,10 @@ class TestContinuationFrames(object): Tests for the relatively complex CONTINUATION frame logic. """ example_request_headers = [ - (':authority', 'example.com'), - (':path', '/'), - (':scheme', 'https'), - (':method', 'GET'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), ] server_config = h2.config.H2Configuration(client_side=False) @@ -371,14 +371,14 @@ class TestContinuationFramesPushPromise(object): PUSH_PROMISE frames. """ example_request_headers = [ - (':authority', 'example.com'), - (':path', '/'), - (':scheme', 'https'), - (':method', 'GET'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), ] example_response_headers = [ - (':status', '200'), - ('server', 'fake-serv/0.1.0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0') ] def _build_continuation_sequence(self, headers, block_size, frame_factory): diff --git a/test/test_config.py b/test/test_config.py index 5f83e362f..8eb7fdc86 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -21,7 +21,7 @@ def test_defaults(self): """ config = h2.config.H2Configuration() assert config.client_side - assert config.header_encoding == 'utf-8' + assert config.header_encoding is None assert isinstance(config.logger, h2.config.DummyLogger) boolean_config_options = [ diff --git a/test/test_h2_upgrade.py b/test/test_h2_upgrade.py index 57981e87c..1d4eac67f 100644 --- a/test/test_h2_upgrade.py +++ b/test/test_h2_upgrade.py @@ -23,14 +23,14 @@ class TestClientUpgrade(object): Tests of the client-side of the HTTP/2 upgrade dance. """ example_request_headers = [ - (u':authority', u'example.com'), - (u':path', u'/'), - (u':scheme', u'https'), - (u':method', u'GET'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), ] example_response_headers = [ - (u':status', u'200'), - (u'server', u'fake-serv/0.1.0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0') ] def test_returns_http2_settings(self, frame_factory): @@ -146,14 +146,14 @@ class TestServerUpgrade(object): Tests of the server-side of the HTTP/2 upgrade dance. """ example_request_headers = [ - (u':authority', u'example.com'), - (u':path', u'/'), - (u':scheme', u'https'), - (u':method', u'GET'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), ] example_response_headers = [ - (u':status', u'200'), - (u'server', u'fake-serv/0.1.0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0') ] server_config = h2.config.H2Configuration(client_side=False) diff --git a/test/test_head_request.py b/test/test_head_request.py index ab7c00918..313bf12a7 100644 --- a/test/test_head_request.py +++ b/test/test_head_request.py @@ -10,16 +10,16 @@ class TestHeadRequest(object): example_request_headers = [ - (u':authority', u'example.com'), - (u':path', u'/'), - (u':scheme', u'https'), - (u':method', u'HEAD'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'HEAD'), ] example_response_headers = [ - (u':status', u'200'), - (u'server', u'fake-serv/0.1.0'), - (u'content_length', u'1'), + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0'), + (b'content_length', b'1'), ] def test_non_zero_content_and_no_body(self, frame_factory): diff --git a/test/test_informational_responses.py b/test/test_informational_responses.py index 72d7f1717..e18c44bcb 100644 --- a/test/test_informational_responses.py +++ b/test/test_informational_responses.py @@ -19,22 +19,22 @@ class TestReceivingInformationalResponses(object): Tests for receiving informational responses. """ example_request_headers = [ - (':authority', 'example.com'), - (':path', '/'), - (':scheme', 'https'), - (':method', 'GET'), - ('expect', '100-continue'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), + (b'expect', b'100-continue'), ] example_informational_headers = [ - (':status', '100'), - ('server', 'fake-serv/0.1.0') + (b':status', b'100'), + (b'server', b'fake-serv/0.1.0') ] example_response_headers = [ - (':status', '200'), - ('server', 'fake-serv/0.1.0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0') ] example_trailers = [ - ('trailer', 'you-bet'), + (b'trailer', b'you-bet'), ] @pytest.mark.parametrize('end_stream', (True, False)) @@ -141,7 +141,7 @@ def test_receiving_multiple_informational_responses(self, assert events[0].stream_id == 1 assert isinstance(events[1], h2.events.InformationalResponseReceived) - assert events[1].headers == [(':status', '101')] + assert events[1].headers == [(b':status', b'101')] assert events[1].stream_id == 1 @pytest.mark.parametrize('end_stream', (True, False)) @@ -216,11 +216,11 @@ class TestSendingInformationalResponses(object): Tests for sending informational responses. """ example_request_headers = [ - (':authority', 'example.com'), - (':path', '/'), - (':scheme', 'https'), - (':method', 'GET'), - ('expect', '100-continue'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), + (b'expect', b'100-continue'), ] unicode_informational_headers = [ (u':status', u'100'), @@ -231,11 +231,11 @@ class TestSendingInformationalResponses(object): (b'server', b'fake-serv/0.1.0') ] example_response_headers = [ - (':status', '200'), - ('server', 'fake-serv/0.1.0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0') ] example_trailers = [ - ('trailer', 'you-bet'), + (b'trailer', b'you-bet'), ] server_config = h2.config.H2Configuration(client_side=False) diff --git a/test/test_interacting_stacks.py b/test/test_interacting_stacks.py index 6bd24b2c0..90776829c 100644 --- a/test/test_interacting_stacks.py +++ b/test/test_interacting_stacks.py @@ -37,16 +37,16 @@ def test_basic_request_response(self): A request issued by hyper-h2 can be responded to by hyper-h2. """ request_headers = [ - (':method', 'GET'), - (':path', '/'), - (':authority', 'example.com'), - (':scheme', 'https'), - ('user-agent', 'test-client/0.1.0'), + (b':method', b'GET'), + (b':path', b'/'), + (b':authority', b'example.com'), + (b':scheme', b'https'), + (b'user-agent', b'test-client/0.1.0'), ] response_headers = [ - (':status', '204'), - ('server', 'test-server/0.1.0'), - ('content-length', '0'), + (b':status', b'204'), + (b'server', b'test-server/0.1.0'), + (b'content-length', b'0'), ] def client(): diff --git a/test/test_invalid_headers.py b/test/test_invalid_headers.py index adc2d8cdd..25e45135e 100644 --- a/test/test_invalid_headers.py +++ b/test/test_invalid_headers.py @@ -55,7 +55,9 @@ class TestInvalidFrameSequences(object): [header for header in base_request_headers if header[0] != ':authority'], ] - server_config = h2.config.H2Configuration(client_side=False) + server_config = h2.config.H2Configuration( + client_side=False, header_encoding='utf-8' + ) @pytest.mark.parametrize('headers', invalid_header_blocks) def test_headers_event(self, frame_factory, headers): @@ -113,7 +115,8 @@ def test_push_promise_skipping_validation(self, frame_factory, headers): """ config = h2.config.H2Configuration( client_side=True, - validate_inbound_headers=False + validate_inbound_headers=False, + header_encoding='utf-8' ) c = h2.connection.H2Connection(config=config) @@ -143,7 +146,9 @@ def test_headers_event_skipping_validation(self, frame_factory, headers): """ config = h2.config.H2Configuration( client_side=False, - validate_inbound_headers=False) + validate_inbound_headers=False, + header_encoding='utf-8' + ) c = h2.connection.H2Connection(config=config) c.receive_data(frame_factory.preamble()) diff --git a/test/test_stream_reset.py b/test/test_stream_reset.py index a842384ab..08886e8e3 100644 --- a/test/test_stream_reset.py +++ b/test/test_stream_reset.py @@ -21,15 +21,15 @@ class TestStreamReset(object): Tests for resetting streams. """ example_request_headers = [ - (':authority', 'example.com'), - (':path', '/'), - (':scheme', 'https'), - (':method', 'GET'), + (b':authority', b'example.com'), + (b':path', b'/'), + (b':scheme', b'https'), + (b':method', b'GET'), ] example_response_headers = [ - (':status', '200'), - ('server', 'fake-serv/0.1.0'), - ('content-length', '0') + (b':status', b'200'), + (b'server', b'fake-serv/0.1.0'), + (b'content-length', b'0') ] def test_reset_stream_keeps_header_state_correct(self, frame_factory): From 19d56ac79fa3fe5a76a33df8299349b3adaa3f79 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Fri, 24 Mar 2017 09:29:08 +0000 Subject: [PATCH 3/3] Changelog for new header encoding default --- HISTORY.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 701cf6400..d53cdbb20 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -19,6 +19,10 @@ API Changes (Backward-Incompatible) - Removed deprecated ``client_side`` and ``header_encoding`` properties from ``H2Connection``. - ``dict`` objects are no longer allowed for user-supplied headers. +- The default header encoding is now ``None``, not ``utf-8``: this means that + all events that carry headers now return those headers as byte strings by + default. The header encoding can be set back to ``utf-8`` to restore the old + behaviour. API Changes (Backward-Compatible) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~