From: Seth Michael Larson <sethmichaellarson@gmail.com>
Date: Sun, 22 Sep 2019 22:37:57 +0000 (-0500)
Subject: Remove ENABLE_CONNECT_PROTOCOL from HTTP/2 Settings (#373)
X-Git-Tag: 0.7.4~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b574230f90f23e49369df442ba374c235d2067b5;p=thirdparty%2Fhttpx.git

Remove ENABLE_CONNECT_PROTOCOL from HTTP/2 Settings (#373)
---

diff --git a/httpx/dispatch/http2.py b/httpx/dispatch/http2.py
index b1efe6a2..361ddf9a 100644
--- a/httpx/dispatch/http2.py
+++ b/httpx/dispatch/http2.py
@@ -3,6 +3,7 @@ import typing
 
 import h2.connection
 import h2.events
+from h2.settings import SettingCodes, Settings
 
 from ..concurrency.base import BaseEvent, BaseTCPStream, ConcurrencyBackend, TimeoutFlag
 from ..config import TimeoutConfig, TimeoutTypes
@@ -65,6 +66,28 @@ class HTTP2Connection:
         await self.stream.close()
 
     def initiate_connection(self) -> None:
+        # Need to set these manually here instead of manipulating via
+        # __setitem__() otherwise the H2Connection will emit SettingsUpdate
+        # frames in addition to sending the undesired defaults.
+        self.h2_state.local_settings = Settings(
+            client=True,
+            initial_values={
+                # Disable PUSH_PROMISE frames from the server since we don't do anything
+                # with them for now.  Maybe when we support caching?
+                SettingCodes.ENABLE_PUSH: 0,
+                # These two are taken from h2 for safe defaults
+                SettingCodes.MAX_CONCURRENT_STREAMS: 100,
+                SettingCodes.MAX_HEADER_LIST_SIZE: 65536,
+            },
+        )
+
+        # Some websites (*cough* Yahoo *cough*) balk at this setting being
+        # present in the initial handshake since it's not defined in the original
+        # RFC despite the RFC mandating ignoring settings you don't know about.
+        del self.h2_state.local_settings[
+            h2.settings.SettingCodes.ENABLE_CONNECT_PROTOCOL
+        ]
+
         self.h2_state.initiate_connection()
         data_to_send = self.h2_state.data_to_send()
         self.stream.write_no_block(data_to_send)
@@ -88,7 +111,6 @@ class HTTP2Connection:
             f"target={request.url.full_path!r} "
             f"headers={headers!r}"
         )
-
         self.h2_state.send_headers(stream_id, headers)
         data_to_send = self.h2_state.data_to_send()
         await self.stream.write(data_to_send, timeout)
diff --git a/tests/dispatch/test_http2.py b/tests/dispatch/test_http2.py
index c286592e..776ad8da 100644
--- a/tests/dispatch/test_http2.py
+++ b/tests/dispatch/test_http2.py
@@ -1,5 +1,9 @@
 import json
 
+import h2.connection
+import h2.events
+from h2.settings import SettingCodes
+
 from httpx import AsyncClient, Client, Response
 
 from .utils import MockHTTP2Backend
@@ -165,3 +169,38 @@ async def test_async_http2_reconnect(backend):
 
     assert response_2.status_code == 200
     assert json.loads(response_2.content) == {"method": "GET", "path": "/2", "body": ""}
+
+
+async def test_http2_settings_in_handshake(backend):
+    backend = MockHTTP2Backend(app=app, backend=backend)
+
+    async with AsyncClient(backend=backend) as client:
+        await client.get("http://example.org")
+
+    h2_conn = backend.server.conn
+
+    assert isinstance(h2_conn, h2.connection.H2Connection)
+    expected_settings = {
+        SettingCodes.HEADER_TABLE_SIZE: 4096,
+        SettingCodes.ENABLE_PUSH: 0,
+        SettingCodes.MAX_CONCURRENT_STREAMS: 100,
+        SettingCodes.INITIAL_WINDOW_SIZE: 65535,
+        SettingCodes.MAX_FRAME_SIZE: 16384,
+        SettingCodes.MAX_HEADER_LIST_SIZE: 65536,
+        # This one's here because h2 helpfully populates remote_settings
+        # with default values even if the peer doesn't send the setting.
+        SettingCodes.ENABLE_CONNECT_PROTOCOL: 0,
+    }
+    assert dict(h2_conn.remote_settings) == expected_settings
+
+    # We don't expect the ENABLE_CONNECT_PROTOCOL to be in the handshake
+    expected_settings.pop(SettingCodes.ENABLE_CONNECT_PROTOCOL)
+
+    assert len(backend.server.settings_changed) == 1
+    settings = backend.server.settings_changed[0]
+
+    assert isinstance(settings, h2.events.RemoteSettingsChanged)
+    assert len(settings.changed_settings) == len(expected_settings)
+    for setting_code, changed_setting in settings.changed_settings.items():
+        assert isinstance(changed_setting, h2.settings.ChangedSetting)
+        assert changed_setting.new_value == expected_settings[setting_code]
diff --git a/tests/dispatch/utils.py b/tests/dispatch/utils.py
index 38c1b24e..0798b31f 100644
--- a/tests/dispatch/utils.py
+++ b/tests/dispatch/utils.py
@@ -41,6 +41,7 @@ class MockHTTP2Server(BaseTCPStream):
         self.close_connection = False
         self.return_data = {}
         self.returning = {}
+        self.settings_changed = []
 
     # TCP stream interface
 
@@ -81,6 +82,8 @@ class MockHTTP2Server(BaseTCPStream):
                 # This will throw an error if the event is for a not-yet created stream
                 elif self.returning[event.stream_id]:
                     self.send_return_data(event.stream_id)
+            elif isinstance(event, h2.events.RemoteSettingsChanged):
+                self.settings_changed.append(event)
 
     async def write(self, data: bytes, timeout) -> None:
         self.write_no_block(data)