Merge pull request #1275 from cdeler/cdeler-split-outbound-cookie-headers

encode/httpx#1214 prefer sending outbound cookies separately to improve headers compression

Author: Kriechi

src/h2/config.py

@@ -117,6 +117,12 @@ class H2Configuration:
         RFC 7540. Defaults to ``True``.
     :type normalize_outbound_headers: ``bool``
 
+    :param split_outbound_cookies: Controls whether the outbound cookie
+        headers are split before sending or not. According to RFC 7540
+        - 8.1.2.5 the outbound header cookie headers may be split to improve
+        headers compression. Default is ``False``.
+    :type split_outbound_cookies: ``bool``
+
     :param validate_inbound_headers: Controls whether the headers received
         by this object are validated against the rules in RFC 7540.
         Disabling this setting will cause inbound header validation to
@@ -148,6 +154,9 @@ class H2Configuration:
     normalize_outbound_headers = _BooleanConfigOption(
         'normalize_outbound_headers'
     )
+    split_outbound_cookies = _BooleanConfigOption(
+        'split_outbound_cookies'
+    )
     validate_inbound_headers = _BooleanConfigOption(
         'validate_inbound_headers'
     )
@@ -160,13 +169,15 @@ def __init__(self,
                  header_encoding=None,
                  validate_outbound_headers=True,
                  normalize_outbound_headers=True,
+                 split_outbound_cookies=False,
                  validate_inbound_headers=True,
                  normalize_inbound_headers=True,
                  logger=None):
         self.client_side = client_side
         self.header_encoding = header_encoding
         self.validate_outbound_headers = validate_outbound_headers
         self.normalize_outbound_headers = normalize_outbound_headers
+        self.split_outbound_cookies = split_outbound_cookies
         self.validate_inbound_headers = validate_inbound_headers
         self.normalize_inbound_headers = normalize_inbound_headers
         self.logger = logger or DummyLogger(__name__)

src/h2/stream.py

@@ -1243,8 +1243,12 @@ def _build_headers_frames(self,
         # We need to lowercase the header names, and to ensure that secure
         # header fields are kept out of compression contexts.
         if self.config.normalize_outbound_headers:
+            # also we may want to split outbound cookies to improve
+            # headers compression
+            should_split_outbound_cookies = self.config.split_outbound_cookies
+
             headers = normalize_outbound_headers(
-                headers, hdr_validation_flags
+                headers, hdr_validation_flags, should_split_outbound_cookies
             )
         if self.config.validate_outbound_headers:
             headers = validate_outbound_headers(

src/h2/utilities.py

@@ -603,14 +603,41 @@ def _combine_cookie_fields(headers, hdr_validation_flags):
         yield NeverIndexedHeaderTuple(b'cookie', cookie_val)
 
 
-def normalize_outbound_headers(headers, hdr_validation_flags):
+def _split_outbound_cookie_fields(headers, hdr_validation_flags):
+    """
+    RFC 7540 § 8.1.2.5 allows for better compression efficiency,
+    to split the Cookie header field into separate header fields
+
+    We want to do it for outbound requests, as we are doing for
+    inbound.
+    """
+    for header in headers:
+        if header[0] in (b'cookie', 'cookie'):
+            needle = b'; ' if isinstance(header[0], bytes) else '; '
+
+            if needle in header[1]:
+                for cookie_val in header[1].split(needle):
+                    if isinstance(header, HeaderTuple):
+                        yield header.__class__(header[0], cookie_val)
+                    else:
+                        yield header[0], cookie_val
+            else:
+                yield header
+        else:
+            yield header
+
+
+def normalize_outbound_headers(headers, hdr_validation_flags, should_split_outbound_cookies):
     """
     Normalizes a header sequence that we are about to send.
 
     :param headers: The HTTP header set.
     :param hdr_validation_flags: An instance of HeaderValidationFlags.
+    :param should_split_outbound_cookies: boolean flag
     """
     headers = _lowercase_header_names(headers, hdr_validation_flags)
+    if should_split_outbound_cookies:
+        headers = _split_outbound_cookie_fields(headers, hdr_validation_flags)
     headers = _strip_surrounding_whitespace(headers, hdr_validation_flags)
     headers = _strip_connection_headers(headers, hdr_validation_flags)
     headers = _secure_headers(headers, hdr_validation_flags)

test/test_basic_logic.py

@@ -9,6 +9,7 @@
 
 import hyperframe
 import pytest
+from hpack import HeaderTuple
 
 import h2.config
 import h2.connection
@@ -789,6 +790,48 @@ def test_headers_are_lowercase(self, frame_factory):
 
         assert c.data_to_send() == expected_frame.serialize()
 
+    def test_outbound_cookie_headers_are_split(self):
+        """
+        We should split outbound cookie headers according to
+        RFC 7540 - 8.1.2.5
+        """
+        cookie_headers = [
+            HeaderTuple('cookie',
+                        'username=John Doe; expires=Thu, 18 Dec 2013 12:00:00 UTC'),
+            ('cookie', 'path=1'),
+            ('cookie', 'test1=val1; test2=val2')
+        ]
+
+        expected_cookie_headers = [
+            HeaderTuple('cookie', 'username=John Doe'),
+            HeaderTuple('cookie', 'expires=Thu, 18 Dec 2013 12:00:00 UTC'),
+            ('cookie', 'path=1'),
+            ('cookie', 'test1=val1'),
+            ('cookie', 'test2=val2'),
+        ]
+
+        client_config = h2.config.H2Configuration(
+            client_side=True,
+            header_encoding='utf-8',
+            split_outbound_cookies=True
+        )
+        server_config = h2.config.H2Configuration(
+            client_side=False,
+            normalize_inbound_headers=False,
+            header_encoding='utf-8'
+        )
+        client = h2.connection.H2Connection(config=client_config)
+        server = h2.connection.H2Connection(config=server_config)
+
+        client.initiate_connection()
+        client.send_headers(1, self.example_request_headers + cookie_headers, end_stream=True)
+
+        e = server.receive_data(client.data_to_send())
+
+        cookie_fields = [(n, v) for n, v in e[1].headers if n == 'cookie']
+
+        assert cookie_fields == expected_cookie_headers
+
     @given(frame_size=integers(min_value=2**14, max_value=(2**24 - 1)))
     @settings(suppress_health_check=[HealthCheck.function_scoped_fixture])
     def test_changing_max_frame_size(self, frame_factory, frame_size):

test/test_invalid_headers.py

@@ -296,7 +296,7 @@ def test_headers_event_skipping_validation(self, frame_factory, headers):
         c.send_headers(1, headers)
 
         # Ensure headers are still normalized.
-        norm_headers = h2.utilities.normalize_outbound_headers(headers, None)
+        norm_headers = h2.utilities.normalize_outbound_headers(headers, None, False)
         f = frame_factory.build_headers_frame(norm_headers)
         assert c.data_to_send() == f.serialize()
 
@@ -322,7 +322,7 @@ def test_push_promise_skipping_validation(self, frame_factory, headers):
 
         # Create push promise frame with normalized headers.
         frame_factory.refresh_encoder()
-        norm_headers = h2.utilities.normalize_outbound_headers(headers, None)
+        norm_headers = h2.utilities.normalize_outbound_headers(headers, None, False)
         pp_frame = frame_factory.build_push_promise_frame(
             stream_id=1, promised_stream_id=2, headers=norm_headers
         )