fix: remove forced follow_redirects from httpx_client_factory calls by jlowin · Pull Request #3496 · PrefectHQ/fastmcp

jlowin · 2026-03-14T16:42:43Z

The StreamableHttpTransport.connect_session was passing follow_redirects=True to custom httpx_client_factory callables with a # type: ignore[call-arg] suppression, since the McpHttpClientFactory protocol does not include follow_redirects as a parameter. This was a protocol violation that forced redirect-following behavior onto custom factories regardless of the caller's intent, and raised concerns about auth credential leakage to cross-origin redirect targets.

The fix removes the forced follow_redirects=True kwarg. httpx already provides built-in protection: its _redirect_headers method automatically strips Authorization headers on cross-origin redirects (any redirect where scheme, host, or port differs). This means Bearer tokens, Basic auth, and other credentials in the Authorization header are never forwarded to a different origin, even when follow_redirects=True is set on the client. The default path via create_mcp_http_client and the verify factory continue to work as before.

# httpx's built-in behavior (verified in new tests):
async with httpx.AsyncClient(follow_redirects=True) as client:
    # If http://server.com/api redirects to http://evil.com/steal,
    # httpx strips the Authorization header before following.
    response = await client.get(
        "http://server.com/api",
        headers={"Authorization": "Bearer secret"},
    )
    # evil.com never sees the Bearer token

Remove the `follow_redirects=True` kwarg that was being forced onto custom httpx_client_factory calls with a type: ignore suppression. The McpHttpClientFactory protocol does not include follow_redirects, so this was a protocol violation. httpx already strips Authorization headers on cross-origin redirects via its _redirect_headers mechanism. 🤖 Co-authored-by: Claude <[email protected]>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 317ffebfa5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T16:45:32Z

            http_client = self.httpx_client_factory(
                headers=headers,
                auth=self.auth,
-                follow_redirects=True,  # type: ignore[call-arg]
                **({"timeout": timeout} if timeout else {}),


Preserve redirect following for custom client factories

Calling httpx_client_factory without follow_redirects=True changes runtime behavior for common factories like httpx.AsyncClient: redirects are now disabled even though the default path (create_mcp_http_client) still follows them. This can break session startup when servers canonicalize transport URLs with 307/302 redirects (for example /mcp/ → /mcp, which is covered in tests/server/test_streamable_http_no_redirect.py), but only for users who provide a custom factory for unrelated concerns (verify/proxy/timeouts).

Useful? React with 👍 / 👎.

marvin-context-protocol · 2026-03-14T16:56:58Z

Test Failure Analysis

Summary: One test in tests/client/test_streamable_http.py timed out on Windows Python 3.10, hitting the 5-second pytest timeout while blocked in asyncio I/O. This failure is unrelated to the PR's changes.

Root Cause: The failure is pre-existing Windows flakiness in the streamable HTTP integration tests — not caused by the removal of follow_redirects=True. Evidence:

The failing tests use the default create_mcp_http_client path, not the custom httpx_client_factory path that this PR modifies
The same tests pass on Ubuntu Python 3.10 and Python 3.13
Recent CI runs on other branches also show intermittent Windows timeouts in similar server-backed HTTP tests
The timeout occurs in the asyncio event loop's select.select (waiting for I/O), with the uvicorn server logging ASGI callable returned without completing response — a symptom of Windows asyncio selector behavior under load

Suggested Solution: This PR's code change is correct and the test failure is not related to it. No changes needed from this PR's perspective. If desired, the Windows flakiness could be addressed separately by adding @pytest.mark.skipif(sys.platform == 'win32', ...) to the affected tests in tests/client/test_streamable_http.py (similar to what TestTimeout already does), or by investigating the root cause of the Windows asyncio hang.

Detailed Analysis

Failing job: Tests: Python 3.10 on windows-latest (Job ID 67078187040)

Log excerpt:

tests\client\test_streamable_http.py +++++++++++++++++++++++++++++++++++ Timeout +++++++++++++++++++++++++++++++++++
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Captured stdout ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
INFO:     127.0.0.1:58965 - "POST /mcp HTTP/1.1" 200 OK
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Captured stderr ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ERROR:    ASGI callable returned without completing response.
~~~~~~~~~~~~~~~~~~~~~~~~~ Stack of MainThread ~~~~~~~~~~~~~~~~~~~~~~~~~~
  ...
  File "C:\...\asyncio\base_events.py", line 1871, in _run_once
    event_list = self._selector.select(timeout)
  File "C:\...\selectors.py", line 324, in select
    r, w, _ = self._select(self._readers, self._writers, [], timeout)
  File "C:\...\selectors.py", line 315, in _select
    r, w, x = select.select(r, w, w, timeout)
+++++++++++++++++++++++++++++++++++ Timeout +++++++++++++++++++++++++++++++++++

The server received and responded to a POST request (200 OK), but the client was stuck waiting for the SSE stream to terminate. The asyncio event loop thread is blocked in select.select — an I/O multiplexer call — indicating the client was waiting for data that never arrived.

Why this isn't caused by the PR: The PR removes follow_redirects=True from the httpx_client_factory call path (lines 167-174 of src/fastmcp/client/transports/http.py). The tests in test_streamable_http.py use StreamableHttpTransport without a custom httpx_client_factory, so they take the create_mcp_http_client code path (lines 181-186), which is untouched by this PR.

Pre-existing flakiness: The TestTimeout class in tests/client/test_streamable_http.py already has a @pytest.mark.skipif(sys.platform == 'win32', reason="Timeout tests are flaky on Windows") annotation, indicating Windows flakiness is a known issue in this test file.

Related Files

src/fastmcp/client/transports/http.py — Modified by this PR; the change (removing follow_redirects=True from factory calls) does not affect the failing test path
tests/client/test_streamable_http.py — Contains the failing tests; unchanged by this PR; already has Windows skip annotations for timeout tests
tests/client/transports/test_no_redirect.py — New test file added by this PR; not involved in the failure

…ss-origin-redirects

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4099ee3963

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T19:39:34Z

+            httpx_client_factory=tracking_factory,
+        )
+        assert transport.httpx_client_factory is tracking_factory
+        assert "follow_redirects" not in received_kwargs


Invoke session setup before checking factory kwargs

This assertion is currently vacuous because StreamableHttpTransport does not call httpx_client_factory during construction; the factory is invoked from connect_session. In this test, received_kwargs therefore remains empty regardless of transport behavior, so the check will pass even if follow_redirects is reintroduced later and the regression this test is meant to prevent comes back.

Useful? React with 👍 / 👎.

httpx already strips Authorization headers on cross-origin redirects, so follow_redirects is safe to keep. Removing it broke redirect handling for users providing custom factories.

The test asserted on received_kwargs but never called connect_session, so the factory was never invoked and the assertion was a no-op.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd432e9681

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-14T20:08:37Z

+        client = create_mcp_http_client(
+            headers={"Authorization": "Bearer secret"},
+        )
+        client._transport = httpx.ASGITransport(app=app)


Use client transport injection that works with proxy mounts

Setting client._transport directly here does not reliably route requests through the ASGI app, because create_mcp_http_client() builds an httpx.AsyncClient with trust_env=True and proxy mounts are selected before _transport for matching URLs. In environments with HTTP_PROXY/HTTPS_PROXY set, this test sends traffic to the proxy instead of the in-memory app (I reproduced a 403 at this assertion), so the new test is flaky/failing rather than validating redirect header stripping.

Useful? React with 👍 / 👎.

…sport

@tool

Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]> Co-authored-by: Marvin Context Protocol <41898282+Marvin Context [email protected]> Co-authored-by: voidborne-d <[email protected]> Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: d 🔹 <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]> Co-authored-by: nightcityblade <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: Bill Easton <[email protected]> Co-authored-by: Sumanshu Nankana <[email protected]> Co-authored-by: Eric Robinson <[email protected]> Co-authored-by: Martim Santos <[email protected]> Co-authored-by: d 🔹 <[email protected]> Co-authored-by: Matthieu B <[email protected]> Co-authored-by: Sascha Buehrle <[email protected]> Co-authored-by: Hakancan <[email protected]> Co-authored-by: nightcityblade <[email protected]> Co-authored-by: Matt Hallowell <[email protected]> Co-authored-by: nate nowack <[email protected]> Co-authored-by: Bill Easton <[email protected]> Co-authored-by: Marcus Shu <[email protected]> Co-authored-by: Rushabh Doshi <[email protected]> Co-authored-by: AIKAWA Shigechika <[email protected]> Co-authored-by: Jeremy Simon <[email protected]> Co-authored-by: Miguel Miranda Dias <[email protected]> Co-authored-by: Anthony James Padavano <[email protected]> Co-authored-by: Mostafa Kamal <[email protected]> Fix auto-close MRE script posting comment without closing (#3386) Fix WorkOS token scope verification bypass 🤖 Generated with Codex (#3407) Fix initialize McpError fallthrough 🤖 Generated with Codex (#3413) Fix transform arg collisions with passthrough params (#3431) Fix get_* returning None when latest version is disabled (#3439) Fix get_* returning None when latest version is disabled (#3421) Fix server lifespan overlap teardown (#3415) Fix $ref output schema object detection regression (#3420) resolved annotations (#3429) Fix async partial callables rejected by iscoroutinefunction (#3438) Fix async partial callables rejected by iscoroutinefunction (#3423) fix: add version to components (#3458) fix: use intent-based flag for OIDC scope patch in load_access_token (#3465) Fixes #3461 fix: normalize Google scope shorthands and surface valid_scopes (#3477) fix: resolve ty 0.0.23 type-checking errors and bump pin (#3481) fix: shield lifespan teardown from cancellation (#3480) fix: forward custom_route endpoints from mounted servers (#3462) fix updates _get_additional_http_routes() to traverse providers, Fixes #3457 fix: remove hardcoded version from CLI help text (#3456) fix: monty 0.0.8 compatibility, drop external_functions from constructor (#3468) fix: task test teardown hanging 5s per test (#3499) Closes #3498 fix: validate workspace path is a directory before cursor install (#3440) Fixes #3426 fix: handle re.error from malformed URI templates in build_regex (#3501) fix: reject empty/OIDC-only required_scopes in AzureProvider (#3503) fix: restrict $ref resolution to local refs only (SSRF/LFI) (#3502) fix warnings and timeouts (#3504) close upgrade check issue when build passes (#3505) Closes #3484 fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767) (#3507) fix: prevent path traversal in skill download (#3493) fix: prefer IdP-granted scopes over client-requested scopes in OAuthProxy (#3492) fix: remove unrelated transform and http.py changes from PR scope fix: remove forced follow_redirects from httpx_client_factory calls (#3496) fix: stop passing follow_redirects to httpx_client_factory fix: restore follow_redirects=True for custom httpx client factories Closes #3509 fix: CSRF double-submit cookie check in consent flow (#3519) fix: validate server names in install commands (#3522) fix: use raw strings for regex in pytest.raises match (#3523) fix: reject refresh tokens used as Bearer access tokens (#3524) fix: route ResourcesAsTools/PromptsAsTools through server middleware (#3495) fix: resolve Pyright "Module is not callable" on @tool, @resource, @prompt decorators (#3540) fix: filter warnings by message in KEY_PREFIX test (#3549) fix: suppress output schema for ToolResult subclass annotations (#3548) fix: increase sleep duration in proxy cache tests (#3567) fix: store absolute token expiry to prevent stale expires_in on reload (#3572) fix: preserve tool properties named 'title' during schema compression (#3582) Fix loopback redirect URI port matching per RFC 8252 §7.3 (#3589) Fix app tool routing: visibility check and middleware propagation (#3591) Fix query parameter serialization to respect OpenAPI explode/style settings (#3595) Fix dev apps form: union types, textarea support, JSON parsing (#3597) fix(google): replace deprecated /oauth2/v1/tokeninfo with /oauth2/v3/userinfo (#3603) fix: resolve EntraOBOToken dependency injection through MultiAuth (#3609) fix(docs): correct misleading stateless_http header (#3622) fix: filesystem provider import machinery (#3626) Closes #3625 (issues 2, 3, 6) fix: recover StdioTransport after subprocess exits (#3630) fix(server): preserve mounted tool task metadata (#3632) fix: scope deprecation warning filter to FastMCPDeprecationWarning (#3649) fix imports, add PrefabAppConfig (#3650) fix: resolve CurrentFastMCP/ctx.fastmcp to child server in mounted background tasks (#3651) Fix blocking docs issues: chart imports, Select API, Rx consistency (#3652) closed by default (#3657) Fix prompt caching middleware missing wrap/unwrap round-trip (#3666) fix: serialize object query params per OpenAPI style/explode rules (#3662) Fixes #2857 fix: HTTP request headers not accessible in background task workers (#3631) fix: restore HTTP headers in worker execution path for background tasks (#3681) fix: strip discriminator after dereferencing schemas (#3682) fix: remove stale ty:ignore directives for ty 0.0.26 (#3684) Fix docs gaps in app provider pages (#3690) fix: dev apps log panel UX improvements (#3698) fix dev server empty string args (#3700)

jlowin added the bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. label Mar 14, 2026

marvin-context-protocol Bot added client Related to the FastMCP client SDK or client-side functionality. http Related to HTTP transport, networking, or web server functionality. high-priority labels Mar 14, 2026

chatgpt-codex-connector Bot reviewed Mar 14, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix/strip-auth-on-cro…

4099ee3

…ss-origin-redirects

chatgpt-codex-connector Bot reviewed Mar 14, 2026

View reviewed changes

jlowin added 2 commits March 14, 2026 15:44

fix: restore follow_redirects=True for custom httpx client factories

0449330

httpx already strips Authorization headers on cross-origin redirects, so follow_redirects is safe to keep. Removing it broke redirect handling for users providing custom factories.

fix: remove vacuous test that never invoked connect_session

bd432e9

The test asserted on received_kwargs but never called connect_session, so the factory was never invoked and the assertion was a no-op.

chatgpt-codex-connector Bot reviewed Mar 14, 2026

View reviewed changes

jlowin added the security Security fixes: input validation, SSRF/LFI prevention, auth hardening, injection defenses. label Mar 15, 2026

fix: use AsyncClient with transport= instead of monkey-patching _tran…

798919d

…sport

jlowin merged commit 226bfb4 into main Mar 15, 2026
7 checks passed

jlowin deleted the fix/strip-auth-on-cross-origin-redirects branch March 15, 2026 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove forced follow_redirects from httpx_client_factory calls#3496

fix: remove forced follow_redirects from httpx_client_factory calls#3496
jlowin merged 5 commits intomainfrom
fix/strip-auth-on-cross-origin-redirects

jlowin commented Mar 14, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Uh oh!

marvin-context-protocol Bot commented Mar 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jlowin commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

marvin-context-protocol Bot commented Mar 14, 2026

Test Failure Analysis

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jlowin commented Mar 14, 2026 •

edited

Loading