1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
|
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:
- 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 <yogita.urade@windriver.com>
---
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
|
