Skip to content

Commit 1f1efbd

Browse files
committed
fix: use Location header for redirect validation instead of next_request
response.next_request is not populated by httpx when follow_redirects=True, causing the redirect protection hook to silently bypass all checks. Parse the Location header directly (matching httpx's own _build_redirect_request flow) and add integration tests via MockTransport to exercise the event hook wiring end-to-end. Github-Issue: #2106
1 parent 5423962 commit 1f1efbd

2 files changed

Lines changed: 42 additions & 16 deletions

File tree

src/mcp/shared/_httpx_utils.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,20 @@ async def _check_redirect(response: httpx.Response, policy: RedirectPolicy) -> N
3838
"""Validate redirect responses against the configured policy.
3939
4040
This is installed as an httpx response event hook. It inspects redirect
41-
responses (3xx with a ``next_request``) and raises
41+
responses (3xx with a ``Location`` header) and raises
4242
:class:`httpx.HTTPStatusError` when the redirect violates *policy*.
4343
4444
Args:
4545
response: The httpx response to check.
4646
policy: The redirect policy to enforce.
4747
"""
48-
if not response.is_redirect or response.next_request is None:
48+
if not response.has_redirect_location:
4949
return
5050

5151
original_url = response.request.url
52-
redirect_url = response.next_request.url
52+
redirect_url = httpx.URL(response.headers["Location"])
53+
if redirect_url.is_relative_url:
54+
redirect_url = original_url.join(redirect_url)
5355

5456
if policy == RedirectPolicy.BLOCK_SCHEME_DOWNGRADE:
5557
if original_url.scheme == "https" and redirect_url.scheme == "http":

tests/shared/test_httpx_utils.py

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,14 @@ async def test_check_redirect_ignores_non_redirect():
5151
await _check_redirect(response, RedirectPolicy.ENFORCE_HTTPS)
5252

5353

54-
async def test_check_redirect_ignores_redirect_without_next_request():
55-
"""Test that redirect responses without next_request are ignored."""
54+
async def test_check_redirect_ignores_redirect_without_location_header():
55+
"""Test that redirect responses without a Location header are ignored."""
5656
response = httpx.Response(
5757
302,
58-
headers={"Location": "http://evil.com"},
5958
request=httpx.Request("GET", "https://example.com"),
6059
)
61-
# next_request is None on a manually constructed response
62-
assert response.next_request is None
60+
# No Location header → has_redirect_location is False
61+
assert not response.has_redirect_location
6362
await _check_redirect(response, RedirectPolicy.BLOCK_SCHEME_DOWNGRADE)
6463

6564

@@ -73,7 +72,6 @@ async def test_block_scheme_downgrade_blocks_https_to_http():
7372
headers={"Location": "http://evil.com"},
7473
request=httpx.Request("GET", "https://example.com"),
7574
)
76-
response.next_request = httpx.Request("GET", "http://evil.com")
7775

7876
with pytest.raises(httpx.HTTPStatusError, match="HTTPS-to-HTTP redirect blocked"):
7977
await _check_redirect(response, RedirectPolicy.BLOCK_SCHEME_DOWNGRADE)
@@ -86,7 +84,6 @@ async def test_block_scheme_downgrade_allows_https_to_https():
8684
headers={"Location": "https://other.com"},
8785
request=httpx.Request("GET", "https://example.com"),
8886
)
89-
response.next_request = httpx.Request("GET", "https://other.com")
9087
await _check_redirect(response, RedirectPolicy.BLOCK_SCHEME_DOWNGRADE)
9188

9289

@@ -97,7 +94,6 @@ async def test_block_scheme_downgrade_allows_http_to_http():
9794
headers={"Location": "http://other.com"},
9895
request=httpx.Request("GET", "http://example.com"),
9996
)
100-
response.next_request = httpx.Request("GET", "http://other.com")
10197
await _check_redirect(response, RedirectPolicy.BLOCK_SCHEME_DOWNGRADE)
10298

10399

@@ -108,7 +104,6 @@ async def test_block_scheme_downgrade_allows_http_to_https():
108104
headers={"Location": "https://other.com"},
109105
request=httpx.Request("GET", "http://example.com"),
110106
)
111-
response.next_request = httpx.Request("GET", "https://other.com")
112107
await _check_redirect(response, RedirectPolicy.BLOCK_SCHEME_DOWNGRADE)
113108

114109

@@ -122,7 +117,6 @@ async def test_enforce_https_blocks_http_target():
122117
headers={"Location": "http://evil.com"},
123118
request=httpx.Request("GET", "https://example.com"),
124119
)
125-
response.next_request = httpx.Request("GET", "http://evil.com")
126120

127121
with pytest.raises(httpx.HTTPStatusError, match="Non-HTTPS redirect blocked"):
128122
await _check_redirect(response, RedirectPolicy.ENFORCE_HTTPS)
@@ -135,7 +129,6 @@ async def test_enforce_https_blocks_http_to_http():
135129
headers={"Location": "http://other.com"},
136130
request=httpx.Request("GET", "http://example.com"),
137131
)
138-
response.next_request = httpx.Request("GET", "http://other.com")
139132

140133
with pytest.raises(httpx.HTTPStatusError, match="Non-HTTPS redirect blocked"):
141134
await _check_redirect(response, RedirectPolicy.ENFORCE_HTTPS)
@@ -148,7 +141,6 @@ async def test_enforce_https_allows_https_target():
148141
headers={"Location": "https://other.com"},
149142
request=httpx.Request("GET", "https://example.com"),
150143
)
151-
response.next_request = httpx.Request("GET", "https://other.com")
152144
await _check_redirect(response, RedirectPolicy.ENFORCE_HTTPS)
153145

154146

@@ -162,5 +154,37 @@ async def test_allow_all_permits_https_to_http():
162154
headers={"Location": "http://evil.com"},
163155
request=httpx.Request("GET", "https://example.com"),
164156
)
165-
response.next_request = httpx.Request("GET", "http://evil.com")
166157
await _check_redirect(response, RedirectPolicy.ALLOW_ALL)
158+
159+
160+
# --- Integration tests (exercise the event hook wiring end-to-end) ---
161+
162+
163+
async def test_redirect_hook_blocks_scheme_downgrade_via_transport():
164+
"""Test that the event hook installed by create_mcp_http_client blocks HTTPS->HTTP."""
165+
166+
def mock_handler(request: httpx.Request) -> httpx.Response:
167+
if str(request.url) == "https://example.com/start":
168+
return httpx.Response(302, headers={"Location": "http://evil.com/stolen"})
169+
return httpx.Response(200, text="OK")
170+
171+
async with create_mcp_http_client() as client:
172+
client._transport = httpx.MockTransport(mock_handler)
173+
174+
with pytest.raises(httpx.HTTPStatusError, match="HTTPS-to-HTTP redirect blocked"):
175+
await client.get("https://example.com/start")
176+
177+
178+
async def test_redirect_hook_allows_safe_redirect_via_transport():
179+
"""Test that the event hook allows HTTPS->HTTPS redirects through the client."""
180+
181+
def mock_handler(request: httpx.Request) -> httpx.Response:
182+
if str(request.url) == "https://example.com/start":
183+
return httpx.Response(302, headers={"Location": "https://example.com/final"})
184+
return httpx.Response(200, text="OK")
185+
186+
async with create_mcp_http_client() as client:
187+
client._transport = httpx.MockTransport(mock_handler)
188+
189+
response = await client.get("https://example.com/start")
190+
assert response.status_code == 200

0 commit comments

Comments
 (0)