From accff72ecc2f6cf5a76d9570198a93ac7c90270e Mon Sep 17 00:00:00 2001
From: Quentin Pradet <quentin.pradet@gmail.com>
Date: Mon, 17 Jun 2024 11:09:06 +0400
Subject: [PATCH] Merge pull request from GHSA-34jh-p97f-mpxf
* Strip Proxy-Authorization header on redirects
* Fix test_retry_default_remove_headers_on_redirect
* Set release date
---
CHANGES.rst | 5 +++++
src/urllib3/util/retry.py | 4 +++-
test/test_retry.py | 6 ++++-
test/with_dummyserver/test_poolmanager.py | 27 ++++++++++++++++++++---
4 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py
index 7a76a4a6ad..0456cceba4 100644
--- a/src/urllib3/util/retry.py
+++ b/src/urllib3/util/retry.py
@@ -189,7 +189,9 @@ class Retry:
RETRY_AFTER_STATUS_CODES = frozenset([413, 429, 503])
#: Default headers to be used for ``remove_headers_on_redirect``
- DEFAULT_REMOVE_HEADERS_ON_REDIRECT = frozenset(["Cookie", "Authorization"])
+ DEFAULT_REMOVE_HEADERS_ON_REDIRECT = frozenset(
+ ["Cookie", "Authorization", "Proxy-Authorization"]
+ )
#: Default maximum backoff time.
DEFAULT_BACKOFF_MAX = 120
diff --git a/test/test_retry.py b/test/test_retry.py
index f71e7acc9e..ac3ce4ca73 100644
--- a/test/test_retry.py
+++ b/test/test_retry.py
@@ -334,7 +334,11 @@ def test_retry_method_not_allowed(self) -> None:
def test_retry_default_remove_headers_on_redirect(self) -> None:
retry = Retry()
- assert retry.remove_headers_on_redirect == {"authorization", "cookie"}
+ assert retry.remove_headers_on_redirect == {
+ "authorization",
+ "proxy-authorization",
+ "cookie",
+ }
def test_retry_set_remove_headers_on_redirect(self) -> None:
retry = Retry(remove_headers_on_redirect=["X-API-Secret"])
diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py
index 4fa9ec850a..af77241d6c 100644
--- a/test/with_dummyserver/test_poolmanager.py
+++ b/test/with_dummyserver/test_poolmanager.py
@@ -144,7 +144,11 @@ def test_redirect_cross_host_remove_headers(self) -> None:
"GET",
f"{self.base_url}/redirect",
fields={"target": f"{self.base_url_alt}/headers"},
- headers={"Authorization": "foo", "Cookie": "foo=bar"},
+ headers={
+ "Authorization": "foo",
+ "Proxy-Authorization": "bar",
+ "Cookie": "foo=bar",
+ },
)
assert r.status == 200
@@ -152,13 +156,18 @@ def test_redirect_cross_host_remove_headers(self) -> None:
data = r.json()
assert "Authorization" not in data
+ assert "Proxy-Authorization" not in data
assert "Cookie" not in data
r = http.request(
"GET",
f"{self.base_url}/redirect",
fields={"target": f"{self.base_url_alt}/headers"},
- headers={"authorization": "foo", "cookie": "foo=bar"},
+ headers={
+ "authorization": "foo",
+ "proxy-authorization": "baz",
+ "cookie": "foo=bar",
+ },
)
assert r.status == 200
@@ -167,6 +176,8 @@ def test_redirect_cross_host_remove_headers(self) -> None:
assert "authorization" not in data
assert "Authorization" not in data
+ assert "proxy-authorization" not in data
+ assert "Proxy-Authorization" not in data
assert "cookie" not in data
assert "Cookie" not in data
@@ -176,7 +187,11 @@ def test_redirect_cross_host_no_remove_headers(self) -> None:
"GET",
f"{self.base_url}/redirect",
fields={"target": f"{self.base_url_alt}/headers"},
- headers={"Authorization": "foo", "Cookie": "foo=bar"},
+ headers={
+ "Authorization": "foo",
+ "Proxy-Authorization": "bar",
+ "Cookie": "foo=bar",
+ },
retries=Retry(remove_headers_on_redirect=[]),
)
@@ -185,6 +200,7 @@ def test_redirect_cross_host_no_remove_headers(self) -> None:
data = r.json()
assert data["Authorization"] == "foo"
+ assert data["Proxy-Authorization"] == "bar"
assert data["Cookie"] == "foo=bar"
def test_redirect_cross_host_set_removed_headers(self) -> None:
@@ -196,6 +212,7 @@ def test_redirect_cross_host_set_removed_headers(self) -> None:
headers={
"X-API-Secret": "foo",
"Authorization": "bar",
+ "Proxy-Authorization": "baz",
"Cookie": "foo=bar",
},
retries=Retry(remove_headers_on_redirect=["X-API-Secret"]),
@@ -207,11 +224,13 @@ def test_redirect_cross_host_set_removed_headers(self) -> None:
assert "X-API-Secret" not in data
assert data["Authorization"] == "bar"
+ assert data["Proxy-Authorization"] == "baz"
assert data["Cookie"] == "foo=bar"
headers = {
"x-api-secret": "foo",
"authorization": "bar",
+ "proxy-authorization": "baz",
"cookie": "foo=bar",
}
r = http.request(
@@ -229,12 +248,14 @@ def test_redirect_cross_host_set_removed_headers(self) -> None:
assert "x-api-secret" not in data
assert "X-API-Secret" not in data
assert data["Authorization"] == "bar"
+ assert data["Proxy-Authorization"] == "baz"
assert data["Cookie"] == "foo=bar"
# Ensure the header argument itself is not modified in-place.
assert headers == {
"x-api-secret": "foo",
"authorization": "bar",
+ "proxy-authorization": "baz",
"cookie": "foo=bar",
}