File CVE-2025-50181-poolmanager-redirects.patch of Package python-urllib3_1
From f05b1329126d5be6de501f9d1e3e36738bc08857 Mon Sep 17 00:00:00 2001
From: Illia Volochii <illia.volochii@gmail.com>
Date: Wed, 18 Jun 2025 16:25:01 +0300
Subject: [PATCH] Merge commit from fork
* Apply Quentin's suggestion
Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>
* Add tests for disabled redirects in the pool manager
* Add a possible fix for the issue with not raised `MaxRetryError`
* Make urllib3 handle redirects instead of JS when JSPI is used
* Fix info in the new comment
* State that redirects with XHR are not controlled by urllib3
* Remove excessive params from new test requests
* Add tests reaching max non-0 redirects
* Test redirects with Emscripten
* Fix `test_merge_pool_kwargs`
* Add a changelog entry
* Parametrize tests
* Drop a fix for Emscripten
* Apply Seth's suggestion to docs
Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>
* Use a minor release instead of the patch one
---------
Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>
Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>
---
CHANGES.rst | 9 ++
docs/reference/contrib/emscripten.rst | 2 +-
dummyserver/app.py | 1 +
src/urllib3/poolmanager.py | 18 +++-
test/contrib/emscripten/test_emscripten.py | 16 ++++
test/test_poolmanager.py | 5 +-
test/with_dummyserver/test_poolmanager.py | 101 +++++++++++++++++++++
7 files changed, 148 insertions(+), 4 deletions(-)
Index: urllib3-1.26.20/src/urllib3/poolmanager.py
===================================================================
--- urllib3-1.26.20.orig/src/urllib3/poolmanager.py
+++ urllib3-1.26.20/src/urllib3/poolmanager.py
@@ -170,6 +170,22 @@ class PoolManager(RequestMethods):
def __init__(self, num_pools=10, headers=None, **connection_pool_kw):
RequestMethods.__init__(self, headers)
+ if "retries" in connection_pool_kw:
+ retries = connection_pool_kw["retries"]
+ if not isinstance(retries, Retry):
+ # When Retry is initialized, raise_on_redirect is based
+ # on a redirect boolean value.
+ # But requests made via a pool manager always set
+ # redirect to False, and raise_on_redirect always ends
+ # up being False consequently.
+ # Here we fix the issue by setting raise_on_redirect to
+ # a value needed by the pool manager without considering
+ # the redirect boolean.
+ raise_on_redirect = retries is not False
+ retries = Retry.from_int(retries, redirect=False)
+ retries.raise_on_redirect = raise_on_redirect
+ connection_pool_kw = connection_pool_kw.copy()
+ connection_pool_kw["retries"] = retries
self.connection_pool_kw = connection_pool_kw
self.pools = RecentlyUsedContainer(num_pools)
@@ -389,7 +405,7 @@ class PoolManager(RequestMethods):
kw["body"] = None
kw["headers"] = HTTPHeaderDict(kw["headers"])._prepare_for_method_change()
- retries = kw.get("retries")
+ retries = kw.get("retries", response.retries)
if not isinstance(retries, Retry):
retries = Retry.from_int(retries, redirect=redirect)
Index: urllib3-1.26.20/test/test_poolmanager.py
===================================================================
--- urllib3-1.26.20.orig/test/test_poolmanager.py
+++ urllib3-1.26.20/test/test_poolmanager.py
@@ -326,9 +326,10 @@ class TestPoolManager(object):
def test_merge_pool_kwargs(self):
"""Assert _merge_pool_kwargs works in the happy case"""
- p = PoolManager(strict=True)
+ retries = retry.Retry(total=100)
+ p = PoolManager(strict=True, retries=retries)
merged = p._merge_pool_kwargs({"new_key": "value"})
- assert {"strict": True, "new_key": "value"} == merged
+ assert {"retries": retries, "strict": True, "new_key": "value"} == merged
def test_merge_pool_kwargs_none(self):
"""Assert false-y values to _merge_pool_kwargs result in defaults"""
Index: urllib3-1.26.20/test/with_dummyserver/test_poolmanager.py
===================================================================
--- urllib3-1.26.20.orig/test/with_dummyserver/test_poolmanager.py
+++ urllib3-1.26.20/test/with_dummyserver/test_poolmanager.py
@@ -82,6 +82,94 @@ class TestPoolManager(HTTPDummyServerTes
assert r.status == 200
assert r.data == b"Dummy server!"
+ @pytest.mark.parametrize(
+ "retries",
+ (0, Retry(total=0), Retry(redirect=0), Retry(total=0, redirect=0)),
+ )
+ def test_redirects_disabled_for_pool_manager_with_0(
+ self, retries: typing.Literal[0] | Retry
+ ) -> None:
+ """
+ Check handling redirects when retries is set to 0 on the pool
+ manager.
+ """
+ with PoolManager(retries=retries) as http:
+ with pytest.raises(MaxRetryError):
+ http.request("GET", f"{self.base_url}/redirect")
+
+ # Setting redirect=True should not change the behavior.
+ with pytest.raises(MaxRetryError):
+ http.request("GET", f"{self.base_url}/redirect", redirect=True)
+
+ # Setting redirect=False should not make it follow the redirect,
+ # but MaxRetryError should not be raised.
+ response = http.request("GET", f"{self.base_url}/redirect", redirect=False)
+ assert response.status == 303
+
+ @pytest.mark.parametrize(
+ "retries",
+ (
+ False,
+ Retry(total=False),
+ Retry(redirect=False),
+ Retry(total=False, redirect=False),
+ ),
+ )
+ def test_redirects_disabled_for_pool_manager_with_false(
+ self, retries: typing.Literal[False] | Retry
+ ) -> None:
+ """
+ Check that setting retries set to False on the pool manager disables
+ raising MaxRetryError and redirect=True does not change the
+ behavior.
+ """
+ with PoolManager(retries=retries) as http:
+ response = http.request("GET", f"{self.base_url}/redirect")
+ assert response.status == 303
+
+ response = http.request("GET", f"{self.base_url}/redirect", redirect=True)
+ assert response.status == 303
+
+ response = http.request("GET", f"{self.base_url}/redirect", redirect=False)
+ assert response.status == 303
+
+ def test_redirects_disabled_for_individual_request(self) -> None:
+ """
+ Check handling redirects when they are meant to be disabled
+ on the request level.
+ """
+ with PoolManager() as http:
+ # Check when redirect is not passed.
+ with pytest.raises(MaxRetryError):
+ http.request("GET", f"{self.base_url}/redirect", retries=0)
+ response = http.request("GET", f"{self.base_url}/redirect", retries=False)
+ assert response.status == 303
+
+ # Check when redirect=True.
+ with pytest.raises(MaxRetryError):
+ http.request(
+ "GET", f"{self.base_url}/redirect", retries=0, redirect=True
+ )
+ response = http.request(
+ "GET", f"{self.base_url}/redirect", retries=False, redirect=True
+ )
+ assert response.status == 303
+
+ # Check when redirect=False.
+ response = http.request(
+ "GET", f"{self.base_url}/redirect", retries=0, redirect=False
+ )
+ assert response.status == 303
+ response = http.request(
+ "GET", f"{self.base_url}/redirect", retries=False, redirect=False
+ )
+ assert response.status == 303
+
+
+ def test_redirect_cross_host_remove_headers(self) -> None:
+ with PoolManager() as http:
+ r = http.request(
+
def test_cross_host_redirect(self):
with PoolManager() as http:
cross_host_location = "%s/echo?a=b" % self.base_url_alt
@@ -136,6 +224,24 @@ class TestPoolManager(HTTPDummyServerTes
pool = http.connection_from_host(self.host, self.port)
assert pool.num_connections == 1
+ # Check when retries are configured for the pool manager.
+ with PoolManager(retries=1) as http:
+ with pytest.raises(MaxRetryError):
+ http.request(
+ "GET",
+ f"{self.base_url}/redirect",
+ fields={"target": f"/redirect?target={self.base_url}/"},
+ )
+
+ # Here we allow more retries for the request.
+ response = http.request(
+ "GET",
+ f"{self.base_url}/redirect",
+ fields={"target": f"/redirect?target={self.base_url}/"},
+ retries=2,
+ )
+ assert response.status == 200
+
def test_redirect_cross_host_remove_headers(self):
with PoolManager() as http:
r = http.request(
