From f05b1329126d5be6de501f9d1e3e36738bc08857 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Wed, 18 Jun 2025 16:25:01 +0300 Subject: [PATCH] Merge commit from fork * Apply Quentin's suggestion Co-authored-by: Quentin Pradet * 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 * Use a minor release instead of the patch one --------- Co-authored-by: Quentin Pradet Co-authored-by: Seth Michael Larson Changes: - skip docs/reference/contrib/emscripten.rst, dummyserver/app.py and test/contrib/emscripten/test_emscripten.py files which are not presented. CVE: CVE-2025-50181 Upstream-Status: Backport [https://github.com/urllib3/urllib3/commit/f05b1329126d5be6de501f9d1e3e36738bc08857] Signed-off-by: Yogita Urade --- src/urllib3/poolmanager.py | 18 +++- test/with_dummyserver/test_poolmanager.py | 101 ++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/urllib3/poolmanager.py b/src/urllib3/poolmanager.py index fb51bf7..a8de7c6 100644 --- a/src/urllib3/poolmanager.py +++ b/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) diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py index 509daf2..f84f169 100644 --- a/test/with_dummyserver/test_poolmanager.py +++ b/test/with_dummyserver/test_poolmanager.py @@ -82,6 +82,89 @@ class TestPoolManager(HTTPDummyServerTestCase): 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_cross_host_redirect(self): with PoolManager() as http: cross_host_location = "%s/echo?a=b" % self.base_url_alt @@ -136,6 +219,24 @@ class TestPoolManager(HTTPDummyServerTestCase): 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( -- 2.40.0