Skip to content

Commit

Permalink
Merge pull request from GHSA-34jh-p97f-mpxf
Browse files Browse the repository at this point in the history
* Strip Proxy-Authorization header on redirects

* Fix test_retry_default_remove_headers_on_redirect

* Set release date
  • Loading branch information
pquentin committed Jun 17, 2024
1 parent 34be4a5 commit accff72
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
2.2.2 (2024-06-17)
==================

- Added the ``Proxy-Authorization`` header to the list of headers to strip from requests when redirecting to a different host. As before, different headers can be set via ``Retry.remove_headers_on_redirect``.

2.2.1 (2024-02-16)
==================

Expand Down
4 changes: 3 additions & 1 deletion src/urllib3/util/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion test/test_retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
27 changes: 24 additions & 3 deletions test/with_dummyserver/test_poolmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,21 +144,30 @@ 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

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
Expand All @@ -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

Expand All @@ -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=[]),
)

Expand All @@ -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:
Expand All @@ -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"]),
Expand All @@ -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(
Expand All @@ -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",
}

Expand Down

0 comments on commit accff72

Please sign in to comment.