Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport GHSA-v845-jxx5-vc9f to 1.26.x #3139

Merged
merged 2 commits into from Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.rst
@@ -1,6 +1,12 @@
Changes
=======

1.26.17 (2023-10-02)
--------------------

* Added the ``Cookie`` 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``.


1.26.16 (2023-05-23)
--------------------

Expand Down
2 changes: 1 addition & 1 deletion src/urllib3/util/retry.py
Expand Up @@ -235,7 +235,7 @@ class Retry(object):
RETRY_AFTER_STATUS_CODES = frozenset([413, 429, 503])

#: Default headers to be used for ``remove_headers_on_redirect``
DEFAULT_REMOVE_HEADERS_ON_REDIRECT = frozenset(["Authorization"])
DEFAULT_REMOVE_HEADERS_ON_REDIRECT = frozenset(["Cookie", "Authorization"])

#: Maximum backoff time.
DEFAULT_BACKOFF_MAX = 120
Expand Down
4 changes: 2 additions & 2 deletions test/test_retry.py
Expand Up @@ -293,12 +293,12 @@ def test_retry_method_not_in_whitelist(self):
def test_retry_default_remove_headers_on_redirect(self):
retry = Retry()

assert list(retry.remove_headers_on_redirect) == ["authorization"]
assert retry.remove_headers_on_redirect == {"authorization", "cookie"}

def test_retry_set_remove_headers_on_redirect(self):
retry = Retry(remove_headers_on_redirect=["X-API-Secret"])

assert list(retry.remove_headers_on_redirect) == ["x-api-secret"]
assert retry.remove_headers_on_redirect == {"x-api-secret"}

@pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")]) # \xb2 = ^2
def test_parse_retry_after_invalid(self, value):
Expand Down
2 changes: 1 addition & 1 deletion test/test_retry_deprecated.py
Expand Up @@ -295,7 +295,7 @@ def test_retry_method_not_in_whitelist(self):
def test_retry_default_remove_headers_on_redirect(self):
retry = Retry()

assert list(retry.remove_headers_on_redirect) == ["authorization"]
assert retry.remove_headers_on_redirect == {"authorization", "cookie"}

def test_retry_set_remove_headers_on_redirect(self):
retry = Retry(remove_headers_on_redirect=["X-API-Secret"])
Expand Down
24 changes: 19 additions & 5 deletions test/with_dummyserver/test_poolmanager.py
Expand Up @@ -141,20 +141,21 @@ def test_redirect_cross_host_remove_headers(self):
"GET",
"%s/redirect" % self.base_url,
fields={"target": "%s/headers" % self.base_url_alt},
headers={"Authorization": "foo"},
headers={"Authorization": "foo", "Cookie": "foo=bar"},
)

assert r.status == 200

data = json.loads(r.data.decode("utf-8"))

assert "Authorization" not in data
assert "Cookie" not in data

r = http.request(
"GET",
"%s/redirect" % self.base_url,
fields={"target": "%s/headers" % self.base_url_alt},
headers={"authorization": "foo"},
headers={"authorization": "foo", "cookie": "foo=bar"},
)

assert r.status == 200
Expand All @@ -163,14 +164,16 @@ def test_redirect_cross_host_remove_headers(self):

assert "authorization" not in data
assert "Authorization" not in data
assert "cookie" not in data
assert "Cookie" not in data

def test_redirect_cross_host_no_remove_headers(self):
with PoolManager() as http:
r = http.request(
"GET",
"%s/redirect" % self.base_url,
fields={"target": "%s/headers" % self.base_url_alt},
headers={"Authorization": "foo"},
headers={"Authorization": "foo", "Cookie": "foo=bar"},
retries=Retry(remove_headers_on_redirect=[]),
)

Expand All @@ -179,14 +182,19 @@ def test_redirect_cross_host_no_remove_headers(self):
data = json.loads(r.data.decode("utf-8"))

assert data["Authorization"] == "foo"
assert data["Cookie"] == "foo=bar"

def test_redirect_cross_host_set_removed_headers(self):
with PoolManager() as http:
r = http.request(
"GET",
"%s/redirect" % self.base_url,
fields={"target": "%s/headers" % self.base_url_alt},
headers={"X-API-Secret": "foo", "Authorization": "bar"},
headers={
"X-API-Secret": "foo",
"Authorization": "bar",
"Cookie": "foo=bar",
},
retries=Retry(remove_headers_on_redirect=["X-API-Secret"]),
)

Expand All @@ -196,12 +204,17 @@ def test_redirect_cross_host_set_removed_headers(self):

assert "X-API-Secret" not in data
assert data["Authorization"] == "bar"
assert data["Cookie"] == "foo=bar"

r = http.request(
"GET",
"%s/redirect" % self.base_url,
fields={"target": "%s/headers" % self.base_url_alt},
headers={"x-api-secret": "foo", "authorization": "bar"},
headers={
"x-api-secret": "foo",
"authorization": "bar",
"cookie": "foo=bar",
},
retries=Retry(remove_headers_on_redirect=["X-API-Secret"]),
)

Expand All @@ -212,6 +225,7 @@ def test_redirect_cross_host_set_removed_headers(self):
assert "x-api-secret" not in data
assert "X-API-Secret" not in data
assert data["Authorization"] == "bar"
assert data["Cookie"] == "foo=bar"

def test_redirect_without_preload_releases_connection(self):
with PoolManager(block=True, maxsize=2) as http:
Expand Down