159 lines
6.2 KiB
Diff
159 lines
6.2 KiB
Diff
From 5ec0de499b9166ca71c65ab04f2a7e4eb0d66fcc Mon Sep 17 00:00:00 2001
|
|
From: Illia Volochii <illia.volochii@gmail.com>
|
|
Date: Thu, 7 May 2026 18:40:31 +0300
|
|
Subject: [PATCH] Merge commit from fork
|
|
|
|
* Remove sensitive headers in proxy pools too
|
|
|
|
* Add a changelog entry
|
|
|
|
* Check retries history in tests
|
|
|
|
Co-authored-by: Copilot <copilot@github.com>
|
|
|
|
---------
|
|
|
|
Co-authored-by: Copilot <copilot@github.com>
|
|
---
|
|
changelog/GHSA-qccp-gfcp-xxvc.bugfix.rst | 3 +
|
|
dummyserver/asgi_proxy.py | 1 +
|
|
src/urllib3/connectionpool.py | 12 ++++
|
|
.../test_proxy_poolmanager.py | 72 +++++++++++++++++++
|
|
4 files changed, 88 insertions(+)
|
|
create mode 100644 changelog/GHSA-qccp-gfcp-xxvc.bugfix.rst
|
|
|
|
diff --git a/changelog/GHSA-qccp-gfcp-xxvc.bugfix.rst b/changelog/GHSA-qccp-gfcp-xxvc.bugfix.rst
|
|
new file mode 100644
|
|
index 0000000000..bac765ea28
|
|
--- /dev/null
|
|
+++ b/changelog/GHSA-qccp-gfcp-xxvc.bugfix.rst
|
|
@@ -0,0 +1,3 @@
|
|
+Fixed HTTP pools created using ``ProxyManager.connection_from_url`` to strip
|
|
+sensitive headers specified in ``Retry.remove_headers_on_redirect`` when
|
|
+redirecting to a different host.
|
|
diff --git a/dummyserver/asgi_proxy.py b/dummyserver/asgi_proxy.py
|
|
index 00c0a1b8b8..3ff1867380 100755
|
|
--- a/dummyserver/asgi_proxy.py
|
|
+++ b/dummyserver/asgi_proxy.py
|
|
@@ -56,6 +56,7 @@ async def absolute_uri(
|
|
client_response = await client.request(
|
|
method=scope["method"],
|
|
url=scope["path"],
|
|
+ params=scope["query_string"].decode(),
|
|
headers=list(scope["headers"]),
|
|
content=await _read_body(receive),
|
|
)
|
|
diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py
|
|
index 9b66218bf1..70fbc5e725 100644
|
|
--- a/src/urllib3/connectionpool.py
|
|
+++ b/src/urllib3/connectionpool.py
|
|
@@ -897,6 +897,18 @@ def urlopen( # type: ignore[override]
|
|
body = None
|
|
headers = HTTPHeaderDict(headers)._prepare_for_method_change()
|
|
|
|
+ # Strip headers marked as unsafe to forward to the redirected location.
|
|
+ # Check remove_headers_on_redirect to avoid a potential network call within
|
|
+ # self.is_same_host() which may use socket.gethostbyname() in the future.
|
|
+ if retries.remove_headers_on_redirect and not self.is_same_host(
|
|
+ redirect_location
|
|
+ ):
|
|
+ new_headers = headers.copy() # type: ignore[union-attr]
|
|
+ for header in headers:
|
|
+ if header.lower() in retries.remove_headers_on_redirect:
|
|
+ new_headers.pop(header, None)
|
|
+ headers = new_headers
|
|
+
|
|
try:
|
|
retries = retries.increment(method, url, response=response, _pool=self)
|
|
except MaxRetryError:
|
|
diff --git a/test/with_dummyserver/test_proxy_poolmanager.py b/test/with_dummyserver/test_proxy_poolmanager.py
|
|
index 4a932c0de4..6921bc6d22 100644
|
|
--- a/test/with_dummyserver/test_proxy_poolmanager.py
|
|
+++ b/test/with_dummyserver/test_proxy_poolmanager.py
|
|
@@ -37,6 +37,7 @@
|
|
SSLError,
|
|
)
|
|
from urllib3.poolmanager import ProxyManager, proxy_from_url
|
|
+from urllib3.util.retry import RequestHistory
|
|
from urllib3.util.ssl_ import create_urllib3_context
|
|
from urllib3.util.timeout import Timeout
|
|
|
|
@@ -300,6 +301,77 @@ def test_cross_host_redirect(self) -> None:
|
|
assert r._pool is not None
|
|
assert r._pool.host != self.http_host_alt
|
|
|
|
+ _sensitive_headers = {
|
|
+ "Authorization": "foo",
|
|
+ "Proxy-Authorization": "bar",
|
|
+ "Cookie": "foo=bar",
|
|
+ }
|
|
+
|
|
+ @pytest.mark.parametrize(
|
|
+ "sensitive_headers",
|
|
+ (_sensitive_headers, {k.lower(): v for k, v in _sensitive_headers.items()}),
|
|
+ ids=("capitalized", "lowercase"),
|
|
+ )
|
|
+ def test_cross_host_redirect_remove_headers_via_proxy_manager(
|
|
+ self, sensitive_headers: dict[str, str]
|
|
+ ) -> None:
|
|
+ headers_url = f"{self.http_url_alt}/headers"
|
|
+ initial_url = f"{self.http_url}/redirect?target={headers_url}"
|
|
+ with proxy_from_url(self.proxy_url) as proxy_mgr:
|
|
+ r = proxy_mgr.request(
|
|
+ "GET", initial_url, headers=sensitive_headers, retries=1
|
|
+ )
|
|
+ assert r.status == 200
|
|
+ assert r.retries is not None
|
|
+ assert r.retries.history == (
|
|
+ RequestHistory(
|
|
+ method="GET",
|
|
+ url=initial_url,
|
|
+ error=None,
|
|
+ status=303,
|
|
+ redirect_location=headers_url,
|
|
+ ),
|
|
+ )
|
|
+ data = r.json()
|
|
+ for header in sensitive_headers:
|
|
+ assert header not in data
|
|
+
|
|
+ @pytest.mark.parametrize(
|
|
+ "sensitive_headers",
|
|
+ (_sensitive_headers, {k.lower(): v for k, v in _sensitive_headers.items()}),
|
|
+ ids=("capitalized", "lowercase"),
|
|
+ )
|
|
+ def test_cross_host_redirect_remove_headers_via_pool(
|
|
+ self, sensitive_headers: dict[str, str]
|
|
+ ) -> None:
|
|
+ headers_url = f"{self.http_url_alt}/headers"
|
|
+ initial_url = f"{self.http_url}/redirect?target={headers_url}"
|
|
+ with proxy_from_url(self.proxy_url) as proxy_mgr:
|
|
+ pool = proxy_mgr.connection_from_url(self.http_url)
|
|
+ r = pool.urlopen(
|
|
+ "GET",
|
|
+ initial_url,
|
|
+ headers=sensitive_headers,
|
|
+ retries=1,
|
|
+ redirect=True,
|
|
+ assert_same_host=False,
|
|
+ preload_content=True,
|
|
+ )
|
|
+ assert r.status == 200
|
|
+ assert r.retries is not None
|
|
+ assert r.retries.history == (
|
|
+ RequestHistory(
|
|
+ method="GET",
|
|
+ url=initial_url,
|
|
+ error=None,
|
|
+ status=303,
|
|
+ redirect_location=headers_url,
|
|
+ ),
|
|
+ )
|
|
+ data = r.json()
|
|
+ for header in sensitive_headers:
|
|
+ assert header not in data
|
|
+
|
|
def test_cross_protocol_redirect(self) -> None:
|
|
with proxy_from_url(self.proxy_url, ca_certs=DEFAULT_CA) as http:
|
|
cross_protocol_location = f"{self.https_url}/echo?a=b"
|