From 2d7a178dd1d1300fcd9e86b9d01581c482365b83 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 09:49:44 +0100 Subject: [PATCH 1/9] Bump minimum HPACK version again. --- HISTORY.rst | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 2d400b826..642bb3d4f 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -12,7 +12,7 @@ API Changes (Backward-Compatible) to unicode. This defaults to UTF-8 for backward compatibility. To disable the decode and use bytes exclusively, set the field to False, None, or the empty string. This affects all headers, including those pushed by servers. -- Bumped the minimum version of HPACK allowed from 2.0 to 2.1. +- Bumped the minimum version of HPACK allowed from 2.0 to 2.2. - Added support for advertising RFC 7838 Alternative services. Bugfixes diff --git a/setup.py b/setup.py index 5e08df308..21d375cb5 100644 --- a/setup.py +++ b/setup.py @@ -62,7 +62,7 @@ ], install_requires=[ 'hyperframe>=3.1, <5, !=4.0.0', - 'hpack>=2.1, <3', + 'hpack>=2.2, <3', ], extras_require={ ':python_version == "2.7" or python_version == "3.3"': ['enum34>=1.0.4, <2'], From c02647ccef009e7ca854e0be3b1d478a78eda3eb Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 10:40:37 +0100 Subject: [PATCH 2/9] Preserve header indexing status when sending headers. --- h2/stream.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/h2/stream.py b/h2/stream.py index 68e280cf9..9941b5a2e 100644 --- a/h2/stream.py +++ b/h2/stream.py @@ -8,6 +8,7 @@ import warnings from enum import Enum, IntEnum +from hpack import HeaderTuple from hyperframe.frame import ( HeadersFrame, ContinuationFrame, DataFrame, WindowUpdateFrame, RstStreamFrame, PushPromiseFrame, AltSvcFrame @@ -993,7 +994,8 @@ def _build_headers_frames(self, """ Helper method to build headers or push promise frames. """ - headers = ((name.lower(), value) for name, value in headers) + # We need to lowercase the header names. + headers = _lowercase_header_names(headers) encoded_headers = encoder.encode(headers) # Slice into blocks of max_outbound_frame_size. Be careful with this: @@ -1055,3 +1057,16 @@ def _track_content_length(self, length, end_stream): if end_stream and expected != actual: raise InvalidBodyLengthError(expected, actual) + + +def _lowercase_header_names(headers): + """ + Given an iterable of header two-tuples, rebuilds that iterable with the + header names lowercased. This generator produces tuples that preserve the + original type of the header tuple for tuple and any ``HeaderTuple``. + """ + for header in headers: + if isinstance(header, HeaderTuple): + yield header.__class__(header[0].lower(), header[1]) + else: + yield (header[0].lower(), header[1]) From a7a2e9e9e34da7c9041727a1cfaa2891e6e3866f Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 10:40:49 +0100 Subject: [PATCH 3/9] Test we preserve header indexing status. --- test/test_header_indexing.py | 121 +++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 test/test_header_indexing.py diff --git a/test/test_header_indexing.py b/test/test_header_indexing.py new file mode 100644 index 000000000..8fb59cf55 --- /dev/null +++ b/test/test_header_indexing.py @@ -0,0 +1,121 @@ +# -*- coding: utf-8 -*- +""" +test_header_indexing.py +~~~~~~~~~~~~~~~~~~~~~~~ + +This module contains tests that use HPACK header tuples that provide additional +metadata to the hpack module about how to encode the headers. +""" +import pytest + +from hpack import HeaderTuple, NeverIndexedHeaderTuple + +import h2.connection + + +def assert_header_blocks_actually_equal(block_a, block_b): + """ + Asserts that two header bocks are really, truly equal, down to the types + of their tuples. Doesn't return anything. + """ + assert len(block_a) == len(block_b) + + for a, b in zip(block_a, block_b): + assert a == b + assert a.__class__ is b.__class__ + + +class TestHeaderIndexing(object): + """ + Test that Hyper-h2 can correctly handle never indexed header fields using + the appropriate hpack data structures. + """ + example_request_headers = [ + HeaderTuple(u':authority', u'example.com'), + HeaderTuple(u':path', u'/'), + HeaderTuple(u':scheme', u'https'), + HeaderTuple(u':method', u'GET'), + ] + bytes_example_request_headers = [ + HeaderTuple(b':authority', b'example.com'), + HeaderTuple(b':path', b'/'), + HeaderTuple(b':scheme', b'https'), + HeaderTuple(b':method', b'GET'), + ] + + extended_request_headers = [ + HeaderTuple(u':authority', u'example.com'), + HeaderTuple(u':path', u'/'), + HeaderTuple(u':scheme', u'https'), + HeaderTuple(u':method', u'GET'), + NeverIndexedHeaderTuple(u'authorization', u'realpassword'), + ] + bytes_extended_request_headers = [ + HeaderTuple(b':authority', b'example.com'), + HeaderTuple(b':path', b'/'), + HeaderTuple(b':scheme', b'https'), + HeaderTuple(b':method', b'GET'), + NeverIndexedHeaderTuple(b'authorization', b'realpassword'), + ] + + @pytest.mark.parametrize( + 'headers', ( + example_request_headers, + bytes_example_request_headers, + extended_request_headers, + bytes_extended_request_headers, + ) + ) + def test_sending_header_tuples(self, headers, frame_factory): + """ + Providing HeaderTuple and HeaderTuple subclasses preserves the metadata + about indexing. + """ + c = h2.connection.H2Connection() + c.initiate_connection() + + # Clear the data, then send headers. + c.clear_outbound_data_buffer() + c.send_headers(1, headers) + + f = frame_factory.build_headers_frame(headers=headers) + assert c.data_to_send() == f.serialize() + + @pytest.mark.parametrize( + 'headers', ( + example_request_headers, + bytes_example_request_headers, + extended_request_headers, + bytes_extended_request_headers, + ) + ) + def test_header_tuples_in_pushes(self, headers, frame_factory): + """ + Proving HeaderTuple and HeaderTuple subclasses to push promises + preserves metadata about indexing. + """ + c = h2.connection.H2Connection(client_side=False) + c.receive_data(frame_factory.preamble()) + + # We can use normal headers for the request. + f = frame_factory.build_headers_frame( + self.example_request_headers + ) + c.receive_data(f.serialize()) + + frame_factory.refresh_encoder() + expected_frame = frame_factory.build_push_promise_frame( + stream_id=1, + promised_stream_id=2, + headers=headers, + flags=['END_HEADERS'], + ) + + c.clear_outbound_data_buffer() + c.push_stream( + stream_id=1, + promised_stream_id=2, + request_headers=headers + ) + + assert c.data_to_send() == expected_frame.serialize() From 529fea23dcf38c386bc86776069ed6d5f4603156 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 11:26:18 +0100 Subject: [PATCH 4/9] Ensure we preserve the header tuple type. --- h2/stream.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/h2/stream.py b/h2/stream.py index 9941b5a2e..08296625e 100644 --- a/h2/stream.py +++ b/h2/stream.py @@ -845,10 +845,7 @@ def receive_push_promise_in_band(self, events[0].pushed_stream_id = promised_stream_id if header_encoding: - headers = [ - (n.decode(header_encoding), v.decode(header_encoding)) - for n, v in headers - ] + headers = list(_decode_headers(headers, header_encoding)) events[0].headers = headers return [], events @@ -891,10 +888,7 @@ def receive_headers(self, headers, end_stream, header_encoding): raise ProtocolError("Trailers must have END_STREAM set") if header_encoding: - headers = [ - (n.decode(header_encoding), v.decode(header_encoding)) - for n, v in headers - ] + headers = list(_decode_headers(headers, header_encoding)) events[0].headers = headers return [], events @@ -1070,3 +1064,20 @@ def _lowercase_header_names(headers): yield header.__class__(header[0].lower(), header[1]) else: yield (header[0].lower(), header[1]) + + +def _decode_headers(headers, encoding): + """ + Given an iterable of header two-tuples and an encoding, decodes those + headers using that encoding while preserving the type of the header tuple. + This ensures that the use of ``HeaderTuple`` is preserved. + """ + for header in headers: + # This function expects to work on decoded headers, which are always + # HeaderTuple objects. + assert isinstance(header, HeaderTuple) + + name, value = header + name = name.decode(encoding) + value = value.decode(encoding) + yield header.__class__(name, value) From 05b3fc7fd45ad2b627dabc1a5d60320b0097519b Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 11:26:29 +0100 Subject: [PATCH 5/9] Test that header tuple types are preseved on receipt --- test/test_header_indexing.py | 194 ++++++++++++++++++++++++++++++++++- 1 file changed, 193 insertions(+), 1 deletion(-) diff --git a/test/test_header_indexing.py b/test/test_header_indexing.py index 8fb59cf55..ea4fa5e01 100644 --- a/test/test_header_indexing.py +++ b/test/test_header_indexing.py @@ -58,6 +58,26 @@ class TestHeaderIndexing(object): NeverIndexedHeaderTuple(b'authorization', b'realpassword'), ] + example_response_headers = [ + HeaderTuple(u':status', u'200'), + HeaderTuple(u'server', u'fake-serv/0.1.0') + ] + bytes_example_response_headers = [ + HeaderTuple(b':status', b'200'), + HeaderTuple(b'server', b'fake-serv/0.1.0') + ] + + extended_response_headers = [ + HeaderTuple(u':status', u'200'), + HeaderTuple(u'server', u'fake-serv/0.1.0'), + NeverIndexedHeaderTuple(u'secure', u'you-bet'), + ] + bytes_extended_response_headers = [ + HeaderTuple(b':status', b'200'), + HeaderTuple(b'server', b'fake-serv/0.1.0'), + NeverIndexedHeaderTuple(b'secure', b'you-bet'), + ] + @pytest.mark.parametrize( 'headers', ( example_request_headers, @@ -91,7 +111,7 @@ def test_sending_header_tuples(self, headers, frame_factory): ) def test_header_tuples_in_pushes(self, headers, frame_factory): """ - Proving HeaderTuple and HeaderTuple subclasses to push promises + Providing HeaderTuple and HeaderTuple subclasses to push promises preserves metadata about indexing. """ c = h2.connection.H2Connection(client_side=False) @@ -119,3 +139,175 @@ def test_header_tuples_in_pushes(self, headers, frame_factory): ) assert c.data_to_send() == expected_frame.serialize() + + @pytest.mark.parametrize( + 'headers,encoding', ( + (example_request_headers, 'utf-8'), + (bytes_example_request_headers, None), + (extended_request_headers, 'utf-8'), + (bytes_extended_request_headers, None), + ) + ) + def test_header_tuples_are_decoded_request(self, + headers, + encoding, + frame_factory): + """ + The indexing status of the header is preserved when emitting + RequestReceived events. + """ + c = h2.connection.H2Connection( + client_side=False, header_encoding=encoding + ) + c.receive_data(frame_factory.preamble()) + + f = frame_factory.build_headers_frame(headers) + data = f.serialize() + events = c.receive_data(data) + + assert len(events) == 1 + event = events[0] + + assert isinstance(event, h2.events.RequestReceived) + assert_header_blocks_actually_equal(headers, event.headers) + + @pytest.mark.parametrize( + 'headers,encoding', ( + (example_response_headers, 'utf-8'), + (bytes_example_response_headers, None), + (extended_response_headers, 'utf-8'), + (bytes_extended_response_headers, None), + ) + ) + def test_header_tuples_are_decoded_response(self, + headers, + encoding, + frame_factory): + """ + The indexing status of the header is preserved when emitting + ResponseReceived events. + """ + c = h2.connection.H2Connection(header_encoding=encoding) + c.initiate_connection() + c.send_headers(stream_id=1, headers=self.example_request_headers) + + f = frame_factory.build_headers_frame(headers) + data = f.serialize() + events = c.receive_data(data) + + assert len(events) == 1 + event = events[0] + + assert isinstance(event, h2.events.ResponseReceived) + assert_header_blocks_actually_equal(headers, event.headers) + + @pytest.mark.parametrize( + 'headers,encoding', ( + (example_response_headers, 'utf-8'), + (bytes_example_response_headers, None), + (extended_response_headers, 'utf-8'), + (bytes_extended_response_headers, None), + ) + ) + def test_header_tuples_are_decoded_info_response(self, + headers, + encoding, + frame_factory): + """ + The indexing status of the header is preserved when emitting + InformationalResponseReceived events. + """ + # Manipulate the headers to send 100 Continue. We need to copy the list + # to avoid breaking the example headers. + headers = headers[:] + if encoding: + headers[0] = HeaderTuple(u':status', u'100') + else: + headers[0] = HeaderTuple(b':status', b'100') + + c = h2.connection.H2Connection(header_encoding=encoding) + c.initiate_connection() + c.send_headers(stream_id=1, headers=self.example_request_headers) + + f = frame_factory.build_headers_frame(headers) + data = f.serialize() + events = c.receive_data(data) + + assert len(events) == 1 + event = events[0] + + assert isinstance(event, h2.events.InformationalResponseReceived) + assert_header_blocks_actually_equal(headers, event.headers) + + @pytest.mark.parametrize( + 'headers,encoding', ( + (example_response_headers, 'utf-8'), + (bytes_example_response_headers, None), + (extended_response_headers, 'utf-8'), + (bytes_extended_response_headers, None), + ) + ) + def test_header_tuples_are_decoded_trailers(self, + headers, + encoding, + frame_factory): + """ + The indexing status of the header is preserved when emitting + TrailersReceived events. + """ + # Manipulate the headers to remove the status, which shouldn't be in + # the trailers. We need to copy the list to avoid breaking the example + # headers. + headers = headers[1:] + + c = h2.connection.H2Connection(header_encoding=encoding) + c.initiate_connection() + c.send_headers(stream_id=1, headers=self.example_request_headers) + f = frame_factory.build_headers_frame(self.example_response_headers) + data = f.serialize() + c.receive_data(data) + + f = frame_factory.build_headers_frame(headers, flags=['END_STREAM']) + data = f.serialize() + events = c.receive_data(data) + + assert len(events) == 2 + event = events[0] + + assert isinstance(event, h2.events.TrailersReceived) + assert_header_blocks_actually_equal(headers, event.headers) + + @pytest.mark.parametrize( + 'headers,encoding', ( + (example_request_headers, 'utf-8'), + (bytes_example_request_headers, None), + (extended_request_headers, 'utf-8'), + (bytes_extended_request_headers, None), + ) + ) + def test_header_tuples_are_decoded_push_promise(self, + headers, + encoding, + frame_factory): + """ + The indexing status of the header is preserved when emitting + PushedStreamReceived events. + """ + c = h2.connection.H2Connection(header_encoding=encoding) + c.initiate_connection() + c.send_headers(stream_id=1, headers=self.example_request_headers) + + f = frame_factory.build_push_promise_frame( + stream_id=1, + promised_stream_id=2, + headers=headers, + flags=['END_HEADERS'], + ) + data = f.serialize() + events = c.receive_data(data) + + assert len(events) == 1 + event = events[0] + + assert isinstance(event, h2.events.PushedStreamReceived) + assert_header_blocks_actually_equal(headers, event.headers) From 01ae820281b16870ebfa125473d8e197bfd27df0 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 11:30:33 +0100 Subject: [PATCH 6/9] Add hpack to intersphinx. --- docs/source/conf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/source/conf.py b/docs/source/conf.py index ec7586213..247561b40 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -264,5 +264,6 @@ # Example configuration for intersphinx: refer to the Python standard library. intersphinx_mapping = { - 'python': ('https://docs.python.org/3.5/', None) + 'python': ('https://docs.python.org/3.5/', None), + 'hpack': ('https://python-hyper.org/hpack/en/stable/', None), } From e8f1d3eb0f23ddc4850985833b4b6a9492f11a0d Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 11:39:19 +0100 Subject: [PATCH 7/9] Update docs to mention HeaderTuple use. --- h2/connection.py | 14 +++++++++++++- h2/events.py | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/h2/connection.py b/h2/connection.py index b4460c1ed..2f099c4bf 100644 --- a/h2/connection.py +++ b/h2/connection.py @@ -563,6 +563,13 @@ def send_headers(self, stream_id, headers, end_stream=False): Clients may send one or two header blocks: one request block, and optionally one trailer block. + If it is important to send HPACK "never indexed" header fields (as + defined in `RFC 7451 Section 7.1.3 + `_), the user may + instead provide headers using the HPACK library's :class:`HeaderTuple + ` and :class:`NeverIndexedHeaderTuple + ` objects. + .. warning:: In HTTP/2, it is mandatory that all the HTTP/2 special headers (that is, ones whose header keys begin with ``:``) appear at the start of the header block, before any normal headers. @@ -571,11 +578,16 @@ def send_headers(self, stream_id, headers, end_stream=False): may fail. For this reason, passing a ``dict`` to ``headers`` is *deprecated*, and will be removed in 3.0. + .. versionchanged:: 2.3.0 + Added support for using :class:`HeaderTuple + ` objects to store headers. + :param stream_id: The stream ID to send the headers on. If this stream does not currently exist, it will be created. :type stream_id: ``int`` :param headers: The request/response headers to send. - :type headers: An iterable of two tuples of bytestrings. + :type headers: An iterable of two tuples of bytestrings or + :class:`HeaderTuple ` objects. :returns: Nothing """ # Check we can open the stream. diff --git a/h2/events.py b/h2/events.py index ab6b37220..e749101b2 100644 --- a/h2/events.py +++ b/h2/events.py @@ -19,6 +19,10 @@ class RequestReceived(object): The RequestReceived event is fired whenever request headers are received. This event carries the HTTP headers for the given request and the stream ID of the new stream. + + .. versionchanged:: 2.3.0 + Changed the type of ``headers`` to :class:`HeaderTuple + `. This has no effect on current users. """ def __init__(self): #: The Stream ID for the stream this request was made on. @@ -38,6 +42,10 @@ class ResponseReceived(object): The ResponseReceived event is fired whenever request headers are received. This event carries the HTTP headers for the given response and the stream ID of the new stream. + + .. versionchanged:: 2.3.0 + Changed the type of ``headers`` to :class:`HeaderTuple + `. This has no effect on current users. """ def __init__(self): #: The Stream ID for the stream this response was made on. @@ -60,6 +68,10 @@ class TrailersReceived(object): ahead of time (e.g. content-length). This event carries the HTTP header fields that form the trailers and the stream ID of the stream on which they were received. + + .. versionchanged:: 2.3.0 + Changed the type of ``headers`` to :class:`HeaderTuple + `. This has no effect on current users. """ def __init__(self): #: The Stream ID for the stream on which these trailers were received. @@ -88,6 +100,10 @@ class InformationalResponseReceived(object): 1XX status code. .. versionadded:: 2.2.0 + + .. versionchanged:: 2.3.0 + Changed the type of ``headers`` to :class:`HeaderTuple + `. This has no effect on current users. """ def __init__(self): #: The Stream ID for the stream this informational response was made From 5f57618d99b737bd43f2efee8fdb37f1a2cbc2a9 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 11:40:37 +0100 Subject: [PATCH 8/9] Changelog for #194. --- HISTORY.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 642bb3d4f..3d50672d2 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -14,6 +14,11 @@ API Changes (Backward-Compatible) string. This affects all headers, including those pushed by servers. - Bumped the minimum version of HPACK allowed from 2.0 to 2.2. - Added support for advertising RFC 7838 Alternative services. +- Allowed users to provide ``hpack.HeaderTuple`` and + ``hpack.NeverIndexedHeaderTuple`` objects to all methods that send headers. +- Changed all events that carry headers to emit ``hpack.HeaderTuple`` and + ``hpack.NeverIndexedHeaderTuple`` instead of plain tuples. This allows users + to maintain header indexing state. Bugfixes ~~~~~~~~ From 04c5ecd332d49ddb2f6b24cffeba6d93824bf5f9 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 21 Apr 2016 11:41:15 +0100 Subject: [PATCH 9/9] Update push docstring as well. --- h2/connection.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/h2/connection.py b/h2/connection.py index 2f099c4bf..dc29d4e27 100644 --- a/h2/connection.py +++ b/h2/connection.py @@ -717,6 +717,13 @@ def push_stream(self, stream_id, promised_stream_id, request_headers): """ Push a response to the client by sending a PUSH_PROMISE frame. + If it is important to send HPACK "never indexed" header fields (as + defined in `RFC 7451 Section 7.1.3 + `_), the user may + instead provide headers using the HPACK library's :class:`HeaderTuple + ` and :class:`NeverIndexedHeaderTuple + ` objects. + :param stream_id: The ID of the stream that this push is a response to. :type stream_id: ``int`` :param promised_stream_id: The ID of the stream that the pushed @@ -724,7 +731,8 @@ def push_stream(self, stream_id, promised_stream_id, request_headers): :type promised_stream_id: ``int`` :param request_headers: The headers of the request that the pushed response will be responding to. - :type request_headers: An iterable of two tuples of bytestrings. + :type request_headers: An iterable of two tuples of bytestrings or + :class:`HeaderTuple ` objects. :returns: Nothing """ if not self.remote_settings.enable_push: