Skip to content

Commit

Permalink
Merge pull request from GHSA-v845-jxx5-vc9f
Browse files Browse the repository at this point in the history
  • Loading branch information
pquentin committed Oct 2, 2023
1 parent 740380c commit 644124e
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 9 deletions.
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
2.0.6 (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``.

2.0.5 (2023-09-20)
==================

Expand Down
3 changes: 3 additions & 0 deletions docs/user-guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ the ``;`` delimited key-value pairs:
print(resp.json())
# {"cookies": {"id": "30", "session": "f3efe9db"}}
Note that the ``Cookie`` header will be stripped if the server redirects to a
different host.

Cookies provided by the server are stored in the ``Set-Cookie`` header:

.. code-block:: python
Expand Down
2 changes: 1 addition & 1 deletion src/urllib3/util/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ 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(["Authorization"])
DEFAULT_REMOVE_HEADERS_ON_REDIRECT = frozenset(["Cookie", "Authorization"])

#: Default maximum backoff time.
DEFAULT_BACKOFF_MAX = 120
Expand Down
4 changes: 2 additions & 2 deletions test/test_retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,12 @@ def test_retry_method_not_allowed(self) -> None:
def test_retry_default_remove_headers_on_redirect(self) -> None:
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) -> None:
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", "\xb2"]) # \xb2 = ^2
def test_parse_retry_after_invalid(self, value: str) -> None:
Expand Down
30 changes: 24 additions & 6 deletions test/with_dummyserver/test_poolmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,20 +141,21 @@ 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"},
headers={"Authorization": "foo", "Cookie": "foo=bar"},
)

assert r.status == 200

data = r.json()

assert "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"},
headers={"authorization": "foo", "cookie": "foo=bar"},
)

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

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) -> None:
with PoolManager() as http:
r = http.request(
"GET",
f"{self.base_url}/redirect",
fields={"target": f"{self.base_url_alt}/headers"},
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) -> None:
data = r.json()

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

def test_redirect_cross_host_set_removed_headers(self) -> None:
with PoolManager() as http:
r = http.request(
"GET",
f"{self.base_url}/redirect",
fields={"target": f"{self.base_url_alt}/headers"},
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,8 +204,13 @@ def test_redirect_cross_host_set_removed_headers(self) -> None:

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

headers = {"x-api-secret": "foo", "authorization": "bar"}
headers = {
"x-api-secret": "foo",
"authorization": "bar",
"cookie": "foo=bar",
}
r = http.request(
"GET",
f"{self.base_url}/redirect",
Expand All @@ -213,9 +226,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["Cookie"] == "foo=bar"

# Ensure the header argument itself is not modified in-place.
assert headers == {"x-api-secret": "foo", "authorization": "bar"}
assert headers == {
"x-api-secret": "foo",
"authorization": "bar",
"cookie": "foo=bar",
}

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

0 comments on commit 644124e

Please sign in to comment.