From d3f8f251910b8cda9322b7a821f54a45b558f9e3 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 6 May 2020 09:36:29 +0100 Subject: [PATCH 1/3] Fix up proxy header behaviour --- httpcore/_async/http_proxy.py | 17 +++++++++++++++-- httpcore/_sync/http_proxy.py | 14 ++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/httpcore/_async/http_proxy.py b/httpcore/_async/http_proxy.py index 5728be872..540193490 100644 --- a/httpcore/_async/http_proxy.py +++ b/httpcore/_async/http_proxy.py @@ -8,6 +8,17 @@ from .connection_pool import AsyncConnectionPool, ResponseByteStream +def merge_headers(default_headers: Headers = None, override_headers: Headers = None) -> Headers: + """ + Append default_headers and override_headers, de-duplicating if a key existing in both cases. + """ + default_headers = [] if default_headers is None else default_headers + override_headers = [] if override_headers is None else override_headers + has_override = set([key.lower() for key, value in override_headers]) + default_headers = [(key, value) for key, value in default_headers if key.lower() not in has_override] + return default_headers + override_headers + + class AsyncHTTPProxy(AsyncConnectionPool): """ A connection pool for making HTTP requests via an HTTP proxy. @@ -105,7 +116,7 @@ async def _forward_request( # [headers] target = b"%b://%b:%d%b" % url url = self.proxy_origin + (target,) - headers = self.proxy_headers + ([] if headers is None else headers) + headers = merge_headers(self.proxy_headers, headers) response = await connection.request( method, url, headers=headers, stream=stream, timeout=timeout @@ -144,8 +155,10 @@ async def _tunnel_request( # [proxy-headers] target = b"%b:%d" % (url[1], url[2]) connect_url = self.proxy_origin + (target,) + connect_headers = [(b'Host', target), (b'Accept', b'*/*')] + connect_headers = merge_headers(connect_headers, self.proxy_headers) proxy_response = await proxy_connection.request( - b"CONNECT", connect_url, headers=self.proxy_headers, timeout=timeout + b"CONNECT", connect_url, headers=connect_headers, timeout=timeout ) proxy_status_code = proxy_response[1] proxy_reason_phrase = proxy_response[2] diff --git a/httpcore/_sync/http_proxy.py b/httpcore/_sync/http_proxy.py index ff3896a69..e345efbb2 100644 --- a/httpcore/_sync/http_proxy.py +++ b/httpcore/_sync/http_proxy.py @@ -8,6 +8,14 @@ from .connection_pool import SyncConnectionPool, ResponseByteStream +def merge_headers(default_headers: Headers = None, override_headers: Headers = None) -> Headers: + default_headers = [] if default_headers is None else default_headers + override_headers = [] if override_headers is None else override_headers + has_override = set([key.lower() for key, value in override_headers]) + default_headers = [(key, value) for key, value in default_headers if key.lower() not in has_override] + return default_headers + override_headers + + class SyncHTTPProxy(SyncConnectionPool): """ A connection pool for making HTTP requests via an HTTP proxy. @@ -105,7 +113,7 @@ def _forward_request( # [headers] target = b"%b://%b:%d%b" % url url = self.proxy_origin + (target,) - headers = self.proxy_headers + ([] if headers is None else headers) + headers = merge_headers(self.proxy_headers, headers) response = connection.request( method, url, headers=headers, stream=stream, timeout=timeout @@ -144,8 +152,10 @@ def _tunnel_request( # [proxy-headers] target = b"%b:%d" % (url[1], url[2]) connect_url = self.proxy_origin + (target,) + connect_headers = [(b'Host', target), (b'Accept', b'*/*')] + connect_headers = merge_headers(connect_headers, self.proxy_headers) proxy_response = proxy_connection.request( - b"CONNECT", connect_url, headers=self.proxy_headers, timeout=timeout + b"CONNECT", connect_url, headers=connect_headers, timeout=timeout ) proxy_status_code = proxy_response[1] proxy_reason_phrase = proxy_response[2] From 37a36143b9279071eee0f31b78e20c0bc05105ec Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 6 May 2020 09:41:24 +0100 Subject: [PATCH 2/3] Linting --- httpcore/_async/http_proxy.py | 12 +++++++++--- httpcore/_sync/http_proxy.py | 15 ++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/httpcore/_async/http_proxy.py b/httpcore/_async/http_proxy.py index 540193490..dbf616306 100644 --- a/httpcore/_async/http_proxy.py +++ b/httpcore/_async/http_proxy.py @@ -8,14 +8,20 @@ from .connection_pool import AsyncConnectionPool, ResponseByteStream -def merge_headers(default_headers: Headers = None, override_headers: Headers = None) -> Headers: +def merge_headers( + default_headers: Headers = None, override_headers: Headers = None +) -> Headers: """ Append default_headers and override_headers, de-duplicating if a key existing in both cases. """ default_headers = [] if default_headers is None else default_headers override_headers = [] if override_headers is None else override_headers has_override = set([key.lower() for key, value in override_headers]) - default_headers = [(key, value) for key, value in default_headers if key.lower() not in has_override] + default_headers = [ + (key, value) + for key, value in default_headers + if key.lower() not in has_override + ] return default_headers + override_headers @@ -155,7 +161,7 @@ async def _tunnel_request( # [proxy-headers] target = b"%b:%d" % (url[1], url[2]) connect_url = self.proxy_origin + (target,) - connect_headers = [(b'Host', target), (b'Accept', b'*/*')] + connect_headers = [(b"Host", target), (b"Accept", b"*/*")] connect_headers = merge_headers(connect_headers, self.proxy_headers) proxy_response = await proxy_connection.request( b"CONNECT", connect_url, headers=connect_headers, timeout=timeout diff --git a/httpcore/_sync/http_proxy.py b/httpcore/_sync/http_proxy.py index e345efbb2..716052ef5 100644 --- a/httpcore/_sync/http_proxy.py +++ b/httpcore/_sync/http_proxy.py @@ -8,11 +8,20 @@ from .connection_pool import SyncConnectionPool, ResponseByteStream -def merge_headers(default_headers: Headers = None, override_headers: Headers = None) -> Headers: +def merge_headers( + default_headers: Headers = None, override_headers: Headers = None +) -> Headers: + """ + Append default_headers and override_headers, de-duplicating if a key existing in both cases. + """ default_headers = [] if default_headers is None else default_headers override_headers = [] if override_headers is None else override_headers has_override = set([key.lower() for key, value in override_headers]) - default_headers = [(key, value) for key, value in default_headers if key.lower() not in has_override] + default_headers = [ + (key, value) + for key, value in default_headers + if key.lower() not in has_override + ] return default_headers + override_headers @@ -152,7 +161,7 @@ def _tunnel_request( # [proxy-headers] target = b"%b:%d" % (url[1], url[2]) connect_url = self.proxy_origin + (target,) - connect_headers = [(b'Host', target), (b'Accept', b'*/*')] + connect_headers = [(b"Host", target), (b"Accept", b"*/*")] connect_headers = merge_headers(connect_headers, self.proxy_headers) proxy_response = proxy_connection.request( b"CONNECT", connect_url, headers=connect_headers, timeout=timeout From b90a6b81eaf094db3f1f4377f4891495165793ea Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 6 May 2020 10:33:30 +0100 Subject: [PATCH 3/3] Drop redundant proxy_headers in tests --- tests/async_tests/test_interfaces.py | 7 +------ tests/sync_tests/test_interfaces.py | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/async_tests/test_interfaces.py b/tests/async_tests/test_interfaces.py index a0fbdaf6d..3539ca55d 100644 --- a/tests/async_tests/test_interfaces.py +++ b/tests/async_tests/test_interfaces.py @@ -188,12 +188,8 @@ async def test_http_proxy( headers = [(b"host", b"example.org")] max_connections = 1 max_keepalive = 2 - # Tunnel requires the host header to be present, - # Forwarding will use the request headers - proxy_headers = headers if proxy_mode == "TUNNEL_ONLY" else None async with httpcore.AsyncHTTPProxy( proxy_server, - proxy_headers=proxy_headers, proxy_mode=proxy_mode, max_connections=max_connections, max_keepalive=max_keepalive, @@ -220,12 +216,11 @@ async def test_proxy_https_requests( ) -> None: method = b"GET" url = (b"https", b"example.org", 443, b"/") - headers = proxy_headers = [(b"host", b"example.org")] + headers = [(b"host", b"example.org")] max_connections = 1 max_keepalive = 2 async with httpcore.AsyncHTTPProxy( proxy_server, - proxy_headers=proxy_headers, proxy_mode=proxy_mode, ssl_context=ca_ssl_context, max_connections=max_connections, diff --git a/tests/sync_tests/test_interfaces.py b/tests/sync_tests/test_interfaces.py index 798060d41..1e4fc9917 100644 --- a/tests/sync_tests/test_interfaces.py +++ b/tests/sync_tests/test_interfaces.py @@ -188,12 +188,8 @@ def test_http_proxy( headers = [(b"host", b"example.org")] max_connections = 1 max_keepalive = 2 - # Tunnel requires the host header to be present, - # Forwarding will use the request headers - proxy_headers = headers if proxy_mode == "TUNNEL_ONLY" else None with httpcore.SyncHTTPProxy( proxy_server, - proxy_headers=proxy_headers, proxy_mode=proxy_mode, max_connections=max_connections, max_keepalive=max_keepalive, @@ -220,12 +216,11 @@ def test_proxy_https_requests( ) -> None: method = b"GET" url = (b"https", b"example.org", 443, b"/") - headers = proxy_headers = [(b"host", b"example.org")] + headers = [(b"host", b"example.org")] max_connections = 1 max_keepalive = 2 with httpcore.SyncHTTPProxy( proxy_server, - proxy_headers=proxy_headers, proxy_mode=proxy_mode, ssl_context=ca_ssl_context, max_connections=max_connections,