Skip to content

Commit 4a4b8fb

Browse files
committed
Only follow method-preserving redirects; resolve unfollowed ones on every path
Review follow-ups for the origin-scoped redirect handling: - A same-origin 301/302/303 answering a POST is no longer followed: httpx2's next-request rules turn those into a body-less GET, which would drop the JSON-RPC message. Only redirects that keep the method (307/308, or any status for a GET) are followed; the rest come back unfollowed like an off-origin redirect and the call fails naming the location. - The standalone GET stream and the resumption GET now handle an unfollowed redirect the way the message POST does: the GET stream logs it and stops instead of spending its reconnection attempts on the same answer, and a resumed request is resolved with the error instead of being left waiting. One helper builds the message for all three. - OAuth authorization-server metadata discovery treats a 3xx from a well-known candidate like a 4xx and tries the next candidate, matching the protected-resource metadata handler, now that these requests see redirect responses directly. - The simple-tool example keeps the 30s/300s timeouts it had before it stopped using the MCP client factory.
1 parent 8080719 commit 4a4b8fb

9 files changed

Lines changed: 151 additions & 35 deletions

File tree

docs/client/transports.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Pass a URL string and you get **Streamable HTTP**, the transport you deploy behi
2929
--8<-- "docs_src/client_transports/tutorial002.py"
3030
```
3131

32-
That is the whole production client. `Client` wraps the URL in `streamable_http_client(...)` for you, on top of an `httpx2.AsyncClient` configured the way MCP needs: a 30-second timeout for connect/write/pool, and a 300-second read timeout because the server may hold a response stream open. Whichever client is underneath, the transport follows a redirect only when it stays on the endpoint's origin (same scheme, host and port, or `http` to `https` on the same host with the default ports), which covers a trailing-slash redirect. A redirect anywhere else is not followed, and the call it answered fails with an `MCPError` naming the location; if that address is the server you meant, use it as the URL.
32+
That is the whole production client. `Client` wraps the URL in `streamable_http_client(...)` for you, on top of an `httpx2.AsyncClient` configured the way MCP needs: a 30-second timeout for connect/write/pool, and a 300-second read timeout because the server may hold a response stream open. Whichever client is underneath, the transport follows a redirect only when it stays on the endpoint's origin (same scheme, host and port, or `http` to `https` on the same host with the default ports) and keeps the request method, which covers a 307/308 trailing-slash redirect. Any other redirect is not followed, and the call it answered fails with an `MCPError` naming the location; if that address is the server you meant, use it as the URL.
3333

3434
!!! check
3535
A `Client` you have constructed is **not** connected. Construction only picks the transport;

examples/servers/simple-tool/mcp_simple_tool/server.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ async def fetch_website(
99
url: str,
1010
) -> list[types.ContentBlock]:
1111
headers = {"User-Agent": "MCP Test Server (github.com/modelcontextprotocol/python-sdk)"}
12-
async with httpx2.AsyncClient(headers=headers, follow_redirects=True) as client:
12+
timeout = httpx2.Timeout(30, read=300)
13+
async with httpx2.AsyncClient(headers=headers, timeout=timeout, follow_redirects=True) as client:
1314
response = await client.get(url)
1415
response.raise_for_status()
1516
return [types.TextContent(type="text", text=response.text)]

src/mcp/client/auth/utils.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,9 @@ async def handle_auth_metadata_response(response: Response) -> tuple[bool, OAuth
230230
return True, asm
231231
except ValidationError: # pragma: no cover
232232
return True, None
233-
elif response.status_code < 400 or response.status_code >= 500:
234-
return False, None # Non-4XX error, stop trying
235-
return True, None
233+
elif 300 <= response.status_code < 500:
234+
return True, None # Not served at this URL (redirects are not followed) - try the next candidate
235+
return False, None # Server error or unexpected status, stop trying
236236

237237

238238
def validate_authorization_response_iss(iss: str | None, oauth_metadata: OAuthMetadata | None) -> None:

src/mcp/client/sse.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,12 @@ async def sse_client(
5353
timeout: HTTP timeout for regular operations (in seconds).
5454
sse_read_timeout: Timeout for SSE read operations (in seconds).
5555
httpx_client_factory: Factory function for creating the httpx2 client. Whichever client it
56-
returns, MCP requests follow a redirect only within the endpoint's origin (same scheme,
57-
host and port, or http to https on the same host with default ports); a redirect
58-
anywhere else is not followed, so connecting fails with `httpx2.HTTPStatusError` for the
59-
redirect response. The client's `follow_redirects` setting is not consulted, and
60-
requests `auth` makes during an MCP request do not follow redirects.
56+
returns, MCP requests follow a redirect only when it stays on the endpoint's origin
57+
(same scheme, host and port, or http to https on the same host with default ports) and
58+
keeps the request method; any other redirect is not followed, so connecting fails with
59+
`httpx2.HTTPStatusError` for the redirect response. The client's `follow_redirects`
60+
setting is not consulted, and requests `auth` makes during an MCP request do not follow
61+
redirects.
6162
auth: Optional httpx2 authentication handler.
6263
on_session_created: Optional callback invoked with the session ID when received.
6364
"""

src/mcp/client/streamable_http.py

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ class ResumptionError(StreamableHTTPError):
6767
"""Raised when resumption request is invalid."""
6868

6969

70+
def _unfollowed_redirect(response: httpx2.Response) -> str | None:
71+
"""Describe a redirect `stream_within_origin` left unfollowed, or None if `response` is not one."""
72+
if response.next_request is None:
73+
return None
74+
location = response.next_request.url
75+
return f"Redirect to {location} not followed; use that URL as the endpoint if it is the intended server"
76+
77+
7078
@dataclass
7179
class RequestContext:
7280
"""Context for a request operation."""
@@ -216,6 +224,10 @@ async def handle_get_stream(self, client: httpx2.AsyncClient, read_stream_writer
216224
headers[LAST_EVENT_ID] = last_event_id
217225

218226
async with sse_within_origin(client, self.url, headers=headers) as event_source:
227+
if (redirect := _unfollowed_redirect(event_source.response)) is not None:
228+
# The same GET would be redirected again, so retrying cannot help.
229+
logger.warning(f"GET stream not opened: {redirect}")
230+
return
219231
event_source.response.raise_for_status()
220232
logger.debug("GET SSE connection established")
221233

@@ -259,6 +271,13 @@ async def _handle_resumption_request(self, ctx: RequestContext) -> None:
259271
original_request_id = ctx.session_message.message.id
260272

261273
async with sse_within_origin(ctx.client, self.url, headers=headers) as event_source:
274+
if (redirect := _unfollowed_redirect(event_source.response)) is not None:
275+
logger.warning(redirect)
276+
assert original_request_id is not None
277+
await self._resolve_abandoned_request(
278+
ctx.read_stream_writer, original_request_id, redirect, code=INVALID_REQUEST
279+
)
280+
return
262281
event_source.response.raise_for_status()
263282
logger.debug("Resumption GET SSE connection established")
264283

@@ -345,18 +364,11 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
345364
)
346365
return
347366

348-
if response.next_request is not None:
349-
# Left unfollowed by stream_within_origin: the location is outside the endpoint's origin.
350-
location = response.next_request.url
351-
logger.warning(
352-
f"Server redirected {self.url} to {location}, outside the endpoint's origin; not followed"
353-
)
367+
if (redirect := _unfollowed_redirect(response)) is not None:
368+
logger.warning(redirect)
354369
if isinstance(message, JSONRPCRequest):
355370
await self._resolve_abandoned_request(
356-
ctx.read_stream_writer,
357-
message.id,
358-
f"Redirect to {location} not followed: it is outside the endpoint's origin",
359-
code=INVALID_REQUEST,
371+
ctx.read_stream_writer, message.id, redirect, code=INVALID_REQUEST
360372
)
361373
return
362374

@@ -671,11 +683,12 @@ async def streamable_http_client(
671683
http_client: Optional pre-configured httpx2.AsyncClient. If None, a default
672684
client with recommended MCP timeouts will be created. To configure headers,
673685
authentication, or other HTTP settings, create an httpx2.AsyncClient and pass it here.
674-
Whichever client is used, MCP requests follow a redirect only within the endpoint's
675-
origin (same scheme, host and port, or http to https on the same host with default
676-
ports); a redirect anywhere else is not followed and the message it answered fails with
677-
an error naming the location. The client's `follow_redirects` setting is not consulted,
678-
and requests its `auth` handler makes during an MCP request do not follow redirects.
686+
Whichever client is used, MCP requests follow a redirect only when it stays on the
687+
endpoint's origin (same scheme, host and port, or http to https on the same host with
688+
default ports) and keeps the request method (307/308); any other redirect is not
689+
followed and the message it answered fails with an error naming the location. The
690+
client's `follow_redirects` setting is not consulted, and requests its `auth` handler
691+
makes during an MCP request do not follow redirects.
679692
terminate_on_close: If True, send a DELETE request to terminate the session when the context exits.
680693
681694
Yields:

src/mcp/shared/_httpx_utils.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,16 @@ async def stream_within_origin(
8787
An MCP transport talks to one configured endpoint, and everything on a request
8888
(headers, auth, body) was configured for that endpoint. A redirect that stays
8989
on the origin of the request just sent (same scheme, host and port, or http to
90-
https on the same host with default ports), such as a trailing-slash
91-
normalisation, is followed using httpx2's own next-request rules. A redirect
92-
anywhere else is not followed: the redirect response itself is yielded, the
93-
way httpx2 hands one back when `follow_redirects` is off, and the caller
94-
treats it as the non-success it is. The client's own `follow_redirects`
95-
setting is not consulted, and requests an `httpx2.Auth` flow makes during
96-
the call are sent the same way, so they do not follow redirects either.
90+
https on the same host with default ports) and keeps the request's method,
91+
such as a 307/308 trailing-slash normalisation, is followed using httpx2's
92+
own next-request rules. Any other redirect is not followed: the redirect
93+
response itself is yielded, the way httpx2 hands one back when
94+
`follow_redirects` is off, and the caller treats it as the non-success it
95+
is. (httpx2 rewrites a POST into a body-less GET for 301/302/303, which
96+
would drop the message, so those count as not followed for anything but a
97+
GET.) The client's own `follow_redirects` setting is not consulted, and
98+
requests an `httpx2.Auth` flow makes during the call are sent the same way,
99+
so they do not follow redirects either.
97100
98101
Raises:
99102
httpx2.TooManyRedirects: More than `client.max_redirects` redirects were followed.
@@ -103,7 +106,11 @@ async def stream_within_origin(
103106
response = await client.send(request, stream=True, follow_redirects=False)
104107
# Set by httpx2, with its own method/body/header rules, only when the response is a redirect.
105108
next_request = response.next_request
106-
if next_request is None or not _within_origin(response.request.url, next_request.url):
109+
if (
110+
next_request is None
111+
or next_request.method != response.request.method
112+
or not _within_origin(response.request.url, next_request.url)
113+
):
107114
try:
108115
yield response
109116
finally:

tests/client/test_auth.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
extract_resource_metadata_from_www_auth,
2525
extract_scope_from_www_auth,
2626
get_client_metadata_scopes,
27+
handle_auth_metadata_response,
2728
handle_registration_response,
2829
is_valid_client_metadata_url,
2930
should_use_client_metadata_url,
@@ -824,6 +825,17 @@ async def test_resource_param_included_with_protected_resource_metadata(self, oa
824825
assert "resource=" in content
825826

826827

828+
@pytest.mark.anyio
829+
@pytest.mark.parametrize(("status", "keep_trying"), [(404, True), (307, True), (500, False)])
830+
async def test_auth_metadata_response_says_whether_to_try_the_next_discovery_url(
831+
status: int, keep_trying: bool
832+
) -> None:
833+
"""SDK-defined: a 4xx or a 3xx (redirects are not followed on these requests) from a discovery
834+
candidate means the metadata is not served there and the next well-known URL is tried; a 5xx
835+
stops discovery."""
836+
assert await handle_auth_metadata_response(httpx2.Response(status)) == (keep_trying, None)
837+
838+
827839
@pytest.mark.parametrize(
828840
("protocol_version", "expected"),
829841
[

tests/client/test_streamable_http.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ async def record(request: httpx2.Request) -> None:
831831

832832
assert exc_info.value.error.code == INVALID_REQUEST
833833
assert exc_info.value.error.message == snapshot(
834-
"Redirect to http://other.example/mcp/ not followed: it is outside the endpoint's origin"
834+
"Redirect to http://other.example/mcp/ not followed; use that URL as the endpoint if it is the intended server"
835835
)
836836
assert [url for url in urls if "other.example" in url] == []
837837

@@ -861,3 +861,57 @@ def handler(request: httpx2.Request) -> httpx2.Response:
861861
assert reply.message.id == 7
862862
assert reply.message.error.code == INVALID_REQUEST
863863
assert urls == ["http://test/mcp", "http://test/mcp"]
864+
865+
866+
@pytest.mark.anyio
867+
async def test_get_stream_gives_up_without_retrying_when_the_endpoint_redirects_elsewhere() -> None:
868+
"""SDK-defined: the standalone GET stream is not opened through a redirect to another origin,
869+
and since the same GET would be redirected again the transport logs it and stops instead of
870+
spending its reconnection attempts."""
871+
gets: list[str] = []
872+
873+
def handler(request: httpx2.Request) -> httpx2.Response:
874+
gets.append(str(request.url))
875+
return httpx2.Response(307, headers={"location": "http://other.example/mcp"})
876+
877+
transport = StreamableHTTPTransport("http://test/mcp")
878+
transport.session_id = "session-1"
879+
send, receive = create_context_streams[SessionMessage | Exception](1)
880+
with anyio.fail_after(5):
881+
async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http:
882+
await transport.handle_get_stream(http, send)
883+
assert gets == ["http://test/mcp"]
884+
send.close()
885+
receive.close()
886+
887+
888+
@pytest.mark.anyio
889+
async def test_resumption_redirected_elsewhere_resolves_that_request_with_an_error() -> None:
890+
"""SDK-defined: a resumption GET answered with a redirect to another origin is not followed;
891+
the resumed request is resolved with an error naming the location rather than left waiting."""
892+
seen: list[tuple[str, str | None]] = []
893+
894+
def handler(request: httpx2.Request) -> httpx2.Response:
895+
seen.append((f"{request.method} {request.url}", request.headers.get("last-event-id")))
896+
return httpx2.Response(307, headers={"location": "http://other.example/mcp"})
897+
898+
with anyio.fail_after(5):
899+
async with (
900+
httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http,
901+
streamable_http_client("http://test/mcp", http_client=http) as (read, write),
902+
):
903+
await write.send(
904+
SessionMessage(
905+
JSONRPCRequest(jsonrpc="2.0", id="resume-1", method="tools/call", params={}),
906+
metadata=ClientMessageMetadata(resumption_token="evt-41"),
907+
)
908+
)
909+
reply = await read.receive()
910+
assert isinstance(reply, SessionMessage)
911+
assert isinstance(reply.message, JSONRPCError)
912+
assert reply.message.id == "resume-1"
913+
assert reply.message.error.code == INVALID_REQUEST
914+
assert reply.message.error.message == snapshot(
915+
"Redirect to http://other.example/mcp not followed; use that URL as the endpoint if it is the intended server"
916+
)
917+
assert seen == [("GET http://test/mcp", "evt-41")]

tests/shared/test_httpx_utils.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,36 @@ async def test_redirect_outside_origin_is_not_followed(location: str):
118118
assert closed == [True]
119119

120120

121+
@pytest.mark.parametrize("status", [301, 302, 303])
122+
async def test_method_changing_redirect_of_a_post_is_not_followed(status: int):
123+
"""httpx2 turns a POST into a body-less GET for 301/302/303, which would drop the message, so a
124+
same-origin redirect with one of those codes is handed back unfollowed (SDK-defined)."""
125+
url = "http://mcp.example/mcp"
126+
client, received, _ = _recording_client({url: (status, "/mcp/")})
127+
128+
async with client, stream_within_origin(client, "POST", url, content=b"payload") as response:
129+
pass
130+
131+
assert response.status_code == status
132+
assert received == [f"POST {url}"]
133+
134+
135+
@pytest.mark.parametrize("status", [301, 302, 303, 307, 308])
136+
async def test_same_origin_redirect_of_a_get_is_followed_for_every_redirect_status(status: int):
137+
"""A GET keeps its method under every redirect status, so the SSE GET follows all of them
138+
within the origin (SDK-defined policy over httpx2's method rules)."""
139+
url = "http://mcp.example/sse"
140+
client, received, _ = _recording_client({url: (status, "/sse/")})
141+
142+
async with client, stream_within_origin(client, "GET", url) as response:
143+
await response.aread()
144+
145+
assert response.status_code == 200
146+
assert received == [f"GET {url}", "GET http://mcp.example/sse/"]
147+
148+
121149
async def test_https_to_http_on_same_host_is_outside_origin():
122-
"""Only the upgrade direction counts as staying on the origin; a downgrade is refused."""
150+
"""Only the upgrade direction counts as staying on the origin; a downgrade is not followed."""
123151
url = "https://mcp.example/mcp"
124152
client, received, _ = _recording_client({url: (302, "http://mcp.example/mcp")})
125153

0 commit comments

Comments
 (0)