From 8af47e7b3f253c32c0f60d522be1833637797270 Mon Sep 17 00:00:00 2001 From: yuwk <1729065730@qq.com> Date: Tue, 1 Sep 2026 10:57:27 +0800 Subject: [PATCH 1/3] =?UTF-8?q?fix(client):=20[CSharp=20SDK/Client]=20?= =?UTF-8?q?=E5=AF=B9=E9=BD=90=20HTTP=20=E6=8E=A2=E6=B5=8B=E5=9B=9E?= =?UTF-8?q?=E9=80=80=E7=AD=96=E7=95=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 仅对 400、404、405 探测失败回退初始化握手。 - 保留 401、403、5xx 的 HTTP 语义且不发起 SSE 请求。 - 覆盖结构化与非结构化探测响应的回归场景。 --- .../AutoDetectingClientSessionTransport.cs | 76 +++++++++---------- .../Client/McpClientImpl.cs | 13 ++-- .../Client/July2026ProtocolFallbackTests.cs | 45 +++++++---- 3 files changed, 74 insertions(+), 60 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs index 7563acd10..c493acbe0 100644 --- a/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs @@ -74,58 +74,45 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can LogUsingStreamableHttp(_name); ActiveTransport = streamableHttpTransport; } - else if (await StreamableHttpClientSessionTransport.TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError) + else if (await StreamableHttpClientSessionTransport.TryReadJsonRpcErrorAsync(response, cancellationToken).ConfigureAwait(false) is { } parsedError && + StreamableHttpClientSessionTransport.ShouldSurfaceJsonRpcErrorAsProtocolException(response.StatusCode, parsedError)) { - // A JSON-RPC error envelope in the body means the peer IS a Streamable HTTP server. - // Adopt it before surfacing the failure so the catch filter leaves the now-owned - // transport alone, and never mask the response by attempting deprecated SSE. + // Recognized modern JSON-RPC error: the peer is Streamable HTTP. Adopt it and surface + // the protocol failure without masking it behind a deprecated SSE GET. LogUsingStreamableHttp(_name); ActiveTransport = streamableHttpTransport; - - if (StreamableHttpClientSessionTransport.ShouldSurfaceJsonRpcErrorAsProtocolException(response.StatusCode, parsedError)) - { - throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError); - } - - // TryReadJsonRpcErrorAsync buffered the content, so this preserves the same response - // body and status without consuming the network stream a second time. - throw await HttpResponseMessageExtensions.CreateHttpRequestExceptionWithBodyAsync(response, cancellationToken).ConfigureAwait(false); + throw McpSessionHandler.CreateRemoteProtocolExceptionFromError(parsedError); } else { - // Non-JSON-RPC error response: either the server doesn't speak MCP at all, or this - // is an older deployment that expects the SSE transport (which establishes its - // protocol via GET /sse rather than POST). Fall back to SSE per the original - // behavior. Capture the underlying error (status + body) before falling back so that, - // if SSE also fails, we can surface the real Streamable HTTP diagnostic to the caller - // instead of dropping it on the floor (see https://github.com/modelcontextprotocol/csharp-sdk/issues/1526). - // This reads the response body a second time for the application/json case, where - // TryReadJsonRpcErrorAsync above already read it. HttpContent buffers after the first - // read, so this returns the same buffered content and is safe (not a second stream - // consumption). For the common non-JSON error responses (415, 405, plain text) - // TryReadJsonRpcErrorAsync returns early on the content type, so there is no double read. + // Unstructured response, or a parsed JSON-RPC error that is not a recognized modern + // signal. Classify the full HTTP response: preserve #1855's server/discover 400/404 + // skip, keep 401/403/5xx (and other non-allowlisted statuses) off SSE, and try SSE + // only for the remaining unrecognized 400/404/405 responses (including a JSON-RPC- + // bodied 405 that is not a recognized modern error). + // + // TryReadJsonRpcErrorAsync may already have buffered application/json content; + // HttpContent returns that buffer, so this is safe (not a second stream consumption). + // For common non-JSON errors (415, 405, plain text) TryReadJsonRpcErrorAsync returns + // early on the content type, so there is no double read. var streamableHttpError = await HttpResponseMessageExtensions.CreateHttpRequestExceptionWithBodyAsync(response, cancellationToken).ConfigureAwait(false); - if (IsDiscoverProbeRejection(message, response.StatusCode)) + // Preserve #1855: unrecognized server/discover 400/404 must reach the initialize + // retry without an intervening GET. Non-allowlisted statuses (401/403/5xx/415/…) + // retain their HTTP semantics without a deprecated GET. + if (IsDiscoverProbeRejection(message, response.StatusCode) || + !ShouldTrySseFallback(response.StatusCode)) { - // The server/discover probe is protocol negotiation, not transport detection. A server - // predating SEP-2575 rejects the session-less POST with 400 (can't parse the request) or - // 404 (requires Mcp-Session-Id on every non-initialize POST) whether it speaks Streamable - // HTTP or SSE, so neither status is evidence about which transport to use. McpClientImpl - // .ConnectAsync treats exactly these two statuses as "initialize-handshake server" and - // immediately retries with initialize on this same transport — and that attempt still - // falls back to SSE, so an SSE-only server is reached one POST later rather than not at - // all. Attempting SSE here instead spends a GET whose result is discarded on every - // connect to a Streamable-HTTP-only server that predates SEP-2575, and logs a "falling - // back to SSE transport" line that misreports settled protocol negotiation as a failure. - LogSkippingSseFallbackForDiscoverProbe(_name, response.StatusCode); - - await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); + if (IsDiscoverProbeRejection(message, response.StatusCode)) + { + LogSkippingSseFallbackForDiscoverProbe(_name, response.StatusCode); + } + throw streamableHttpError; } + // Try SSE for the remaining unrecognized 400/404/405 responses. LogStreamableHttpFailed(_name, response.StatusCode); - await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); await InitializeSseTransportAsync(message, streamableHttpError, cancellationToken).ConfigureAwait(false); } @@ -133,7 +120,7 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can catch when (ActiveTransport is null) { // Only dispose the Streamable HTTP transport when we didn't adopt it. If we set - // ActiveTransport above (success path OR structured-error path), the transport's + // ActiveTransport above (success path OR recognized-error path), the transport's // lifetime is owned by the outer transport from this point on. await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); throw; @@ -205,6 +192,15 @@ public async ValueTask DisposeAsync() } } + /// + /// Spec allowlist for HTTP→SSE transport fallback: only 400, 404, or 405 may indicate an older + /// SSE-only deployment. Authentication, authorization, and server errors must not trigger a GET. + /// + private static bool ShouldTrySseFallback(HttpStatusCode statusCode) => + statusCode is HttpStatusCode.BadRequest + or HttpStatusCode.NotFound + or HttpStatusCode.MethodNotAllowed; + [LoggerMessage(Level = LogLevel.Debug, Message = "{EndpointName} attempting to connect using Streamable HTTP transport.")] private partial void LogAttemptingStreamableHttp(string endpointName); diff --git a/src/ModelContextProtocol.Core/Client/McpClientImpl.cs b/src/ModelContextProtocol.Core/Client/McpClientImpl.cs index d1f2a9d7a..6b2120b40 100644 --- a/src/ModelContextProtocol.Core/Client/McpClientImpl.cs +++ b/src/ModelContextProtocol.Core/Client/McpClientImpl.cs @@ -381,14 +381,17 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default) fallbackToInitialize = true; } catch (HttpRequestException ex) when ( - ex.GetStatusCode() is HttpStatusCode.BadRequest or HttpStatusCode.NotFound) + ex.GetStatusCode() is HttpStatusCode.BadRequest + or HttpStatusCode.NotFound + or HttpStatusCode.MethodNotAllowed) { // A server predating SEP-2575 can reject the session-less server/discover POST at the // HTTP layer instead of with a JSON-RPC error: 400 when it cannot parse the request, - // 404 when it requires Mcp-Session-Id on every non-initialize POST. A 400 carrying a - // structured JSON-RPC error is surfaced as McpProtocolException and handled above, so - // anything reaching here is plain or empty. Either way this is an initialize-handshake - // server, so fall back. Other statuses stay uncaught and surface to the caller. + // 404 when it requires Mcp-Session-Id on every non-initialize POST, and 405 when it + // does not accept POST at this endpoint at all. A 400 carrying a structured JSON-RPC + // error is surfaced as McpProtocolException and handled above, so anything reaching + // here is plain or empty. Either way this is an initialize-handshake server, so fall + // back. Other statuses stay uncaught and surface to the caller. fallbackToInitialize = true; } catch (OperationCanceledException) when (probeCts.IsCancellationRequested && !initializationCts.IsCancellationRequested) diff --git a/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs b/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs index 557dc5655..1b504fb86 100644 --- a/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs @@ -214,13 +214,16 @@ public void DiscoverProbeTimeout_Setter_Accepts_PositiveAndInfiniteValues() [InlineData(HttpStatusCode.NotFound, HttpTransportMode.AutoDetect)] [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.AutoDetect)] public async Task Client_OnFallbackHttpStatusFromProbe_FallsBackTo_Initialize( HttpStatusCode status, HttpTransportMode transportMode) { // A server predating SEP-2575 can reject the session-less server/discover probe at the HTTP layer // rather than with a JSON-RPC error: 404 when it requires Mcp-Session-Id on every non-initialize - // POST, or a plain/empty 400 when it cannot parse the request. Both are initialize-handshake - // servers, so the connect must fall back instead of failing. + // POST, a plain/empty 400 when it cannot parse the request, or 405 when the endpoint rejects + // the probe method. All three are initialize-handshake servers, so the connect must fall back + // instead of failing. var ct = TestContext.Current.CancellationToken; var initializeReceived = false; @@ -240,17 +243,22 @@ public async Task Client_OnFallbackHttpStatusFromProbe_FallsBackTo_Initialize( } [Theory] - [InlineData(HttpTransportMode.StreamableHttp)] - [InlineData(HttpTransportMode.AutoDetect)] - public async Task Client_OnStructuredInvalidRequestFromHttpProbe_FallsBackTo_Initialize( - HttpTransportMode transportMode) + [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.NotFound, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.NotFound, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.AutoDetect)] + public async Task Client_OnStructuredFallbackHttpStatusFromProbe_FallsBackTo_Initialize( + HttpStatusCode status, HttpTransportMode transportMode) { var ct = TestContext.Current.CancellationToken; var initializeReceived = false; using var mockHttpHandler = new MockHttpHandler(); using var httpClient = new HttpClient(mockHttpHandler); - mockHttpHandler.RequestHandler = CreateStructuredInvalidRequestProbeServer( + mockHttpHandler.RequestHandler = CreateStructuredProbeRejectingServer( + status, () => initializeReceived = true); await using var transport = CreateTransport(httpClient, transportMode); @@ -265,19 +273,21 @@ public async Task Client_OnStructuredInvalidRequestFromHttpProbe_FallsBackTo_Ini [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.Unauthorized, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.AutoDetect)] public async Task Client_OnOtherHttpErrorFromProbe_Surfaces_NoFallback( HttpStatusCode status, HttpTransportMode transportMode) { - // Only 400 and 404 are read as "this server needs the initialize handshake". Any other HTTP failure - // is a genuine transport error and must surface, so callers are not handed a misleading downstream - // error. Guards the deliberate narrowing of the status filter. + // Only 400, 404, and 405 indicate that the server needs the initialize handshake. Authentication + // and server failures must surface directly, without probing deprecated SSE or attempting initialize. var ct = TestContext.Current.CancellationToken; var initializeReceived = false; + var sseRequested = false; using var mockHttpHandler = new MockHttpHandler(); using var httpClient = new HttpClient(mockHttpHandler); mockHttpHandler.RequestHandler = CreateProbeRejectingServer( - status, "nope", () => initializeReceived = true); + status, "nope", () => initializeReceived = true, () => sseRequested = true); await using var transport = CreateTransport(httpClient, transportMode); @@ -288,6 +298,7 @@ await Assert.ThrowsAnyAsync(async () => }); Assert.False(initializeReceived); + Assert.False(sseRequested); } private HttpClientTransport CreateTransport(HttpClient httpClient, HttpTransportMode transportMode) @@ -303,13 +314,17 @@ private HttpClientTransport CreateTransport(HttpClient httpClient, HttpTransport /// and, if the client falls back, completes an initialize handshake at 2025-11-25. /// private static Func> CreateProbeRejectingServer( - HttpStatusCode probeStatus, string probeBody, Action onInitialize) + HttpStatusCode probeStatus, string probeBody, Action onInitialize, Action? onSseRequest = null) => async request => { // The server offers no standalone SSE stream, which the spec permits. // net472 does not populate a default Content, so every response sets one explicitly. if (request.Method == HttpMethod.Get) + { + // Track accidental AutoDetect fallback for non-allowlisted HTTP failures. + onSseRequest?.Invoke(); return EmptyResponse(HttpStatusCode.MethodNotAllowed); + } var body = await request.Content!.ReadAsStringAsync(); using var doc = JsonDocument.Parse(body); @@ -339,8 +354,8 @@ private static Func> CreateProbeRe } }; - private static Func> CreateStructuredInvalidRequestProbeServer( - Action onInitialize) + private static Func> CreateStructuredProbeRejectingServer( + HttpStatusCode probeStatus, Action onInitialize) => async request => { if (request.Method == HttpMethod.Get) @@ -356,7 +371,7 @@ private static Func> CreateStructu var id = doc.RootElement.GetProperty("id").GetRawText(); var error = "{\"jsonrpc\":\"2.0\",\"id\":" + id + ",\"error\":{\"code\":-32600,\"message\":\"Mcp-Session-Id header is required\"}}"; - return new HttpResponseMessage(HttpStatusCode.BadRequest) + return new HttpResponseMessage(probeStatus) { Content = new StringContent(error, Encoding.UTF8, "application/json"), }; From 4584416e05ca7574b949154a5e0bee69788e8ade Mon Sep 17 00:00:00 2001 From: yuwk <1729065730@qq.com> Date: Wed, 9 Sep 2026 18:57:58 +0800 Subject: [PATCH 2/3] fix(client): remove 405 from discover-probe initialize fallback 405 means the POST endpoint rejected the request method, so retrying initialize over the same transport is not useful. The spec's 405 handling is the AutoDetect transport's SSE fallback. Remove MethodNotAllowed from McpClientImpl's HTTP-layer fallback catch and route the regression coverage through the AutoDetect test matrix. Also realign three AutoDetect transport tests that still encoded the pre-allowlist 'always fall back to SSE' behavior (403/415), switching them to an allowlisted 404 so they continue to exercise the dual-failure surface path. --- .../Client/McpClientImpl.cs | 18 +++-- .../Client/July2026ProtocolFallbackTests.cs | 77 ++++++++++++++++--- .../HttpClientTransportAutoDetectTests.cs | 49 ++++++------ 3 files changed, 103 insertions(+), 41 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/McpClientImpl.cs b/src/ModelContextProtocol.Core/Client/McpClientImpl.cs index 6b2120b40..768c24583 100644 --- a/src/ModelContextProtocol.Core/Client/McpClientImpl.cs +++ b/src/ModelContextProtocol.Core/Client/McpClientImpl.cs @@ -382,16 +382,18 @@ public async Task ConnectAsync(CancellationToken cancellationToken = default) } catch (HttpRequestException ex) when ( ex.GetStatusCode() is HttpStatusCode.BadRequest - or HttpStatusCode.NotFound - or HttpStatusCode.MethodNotAllowed) + or HttpStatusCode.NotFound) { // A server predating SEP-2575 can reject the session-less server/discover POST at the - // HTTP layer instead of with a JSON-RPC error: 400 when it cannot parse the request, - // 404 when it requires Mcp-Session-Id on every non-initialize POST, and 405 when it - // does not accept POST at this endpoint at all. A 400 carrying a structured JSON-RPC - // error is surfaced as McpProtocolException and handled above, so anything reaching - // here is plain or empty. Either way this is an initialize-handshake server, so fall - // back. Other statuses stay uncaught and surface to the caller. + // HTTP layer instead of with a JSON-RPC error: 400 when it cannot parse the request, or + // 404 when it requires Mcp-Session-Id on every non-initialize POST. A 400 carrying a + // structured JSON-RPC error is surfaced as McpProtocolException and handled above, so + // anything reaching here is plain or empty. Either way this is an initialize-handshake + // server, so fall back. A 405 means the POST endpoint rejected the request method + // entirely, so retrying initialize over the same transport is not useful; the spec's 405 + // handling is the AutoDetect transport's fallback to SSE, and in explicit Streamable HTTP + // mode a 405 surfaces to the caller. Other statuses stay uncaught and surface to the + // caller. fallbackToInitialize = true; } catch (OperationCanceledException) when (probeCts.IsCancellationRequested && !initializationCts.IsCancellationRequested) diff --git a/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs b/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs index 1b504fb86..70efac14a 100644 --- a/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs @@ -214,16 +214,16 @@ public void DiscoverProbeTimeout_Setter_Accepts_PositiveAndInfiniteValues() [InlineData(HttpStatusCode.NotFound, HttpTransportMode.AutoDetect)] [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.AutoDetect)] - [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.StreamableHttp)] - [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.AutoDetect)] public async Task Client_OnFallbackHttpStatusFromProbe_FallsBackTo_Initialize( HttpStatusCode status, HttpTransportMode transportMode) { // A server predating SEP-2575 can reject the session-less server/discover probe at the HTTP layer // rather than with a JSON-RPC error: 404 when it requires Mcp-Session-Id on every non-initialize - // POST, a plain/empty 400 when it cannot parse the request, or 405 when the endpoint rejects - // the probe method. All three are initialize-handshake servers, so the connect must fall back - // instead of failing. + // POST, or a plain/empty 400 when it cannot parse the request. Both are initialize-handshake + // servers, so the connect must fall back instead of failing. (405 is deliberately excluded: the + // POST endpoint rejecting the request method does not mean initialize will succeed over the same + // transport, and the spec routes 405 to the AutoDetect transport's SSE fallback — see + // Client_On405FromProbe_DoesNotFallBackTo_Initialize.) var ct = TestContext.Current.CancellationToken; var initializeReceived = false; @@ -247,8 +247,6 @@ public async Task Client_OnFallbackHttpStatusFromProbe_FallsBackTo_Initialize( [InlineData(HttpStatusCode.BadRequest, HttpTransportMode.AutoDetect)] [InlineData(HttpStatusCode.NotFound, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.NotFound, HttpTransportMode.AutoDetect)] - [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.StreamableHttp)] - [InlineData(HttpStatusCode.MethodNotAllowed, HttpTransportMode.AutoDetect)] public async Task Client_OnStructuredFallbackHttpStatusFromProbe_FallsBackTo_Initialize( HttpStatusCode status, HttpTransportMode transportMode) { @@ -269,6 +267,66 @@ public async Task Client_OnStructuredFallbackHttpStatusFromProbe_FallsBackTo_Ini Assert.Equal(McpProtocolVersions.November2025ProtocolVersion, client.NegotiatedProtocolVersion); } + [Theory] + [InlineData(HttpTransportMode.StreamableHttp, false)] + [InlineData(HttpTransportMode.AutoDetect, true)] + public async Task Client_On405FromProbe_DoesNotFallBackTo_Initialize( + HttpTransportMode transportMode, bool expectSseAttempt) + { + // 405 means the POST endpoint rejected the request method, so retrying initialize over the same + // transport is not useful. The spec routes 405 to the AutoDetect transport's SSE fallback: in + // Streamable HTTP mode the 405 surfaces directly, and in AutoDetect mode the client attempts the + // deprecated SSE GET instead of initialize. Neither path may attempt initialize. + var ct = TestContext.Current.CancellationToken; + var initializeReceived = false; + var sseRequested = false; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + mockHttpHandler.RequestHandler = CreateProbeRejectingServer( + HttpStatusCode.MethodNotAllowed, "Invalid session ID", + () => initializeReceived = true, () => sseRequested = true); + + await using var transport = CreateTransport(httpClient, transportMode); + + await Assert.ThrowsAnyAsync(async () => + { + await using var client = await McpClient.CreateAsync(transport, new McpClientOptions(), + loggerFactory: LoggerFactory, cancellationToken: ct); + }); + + Assert.False(initializeReceived); + Assert.Equal(expectSseAttempt, sseRequested); + } + + [Theory] + [InlineData(HttpTransportMode.StreamableHttp)] + [InlineData(HttpTransportMode.AutoDetect)] + public async Task Client_OnStructured405FromProbe_DoesNotFallBackTo_Initialize( + HttpTransportMode transportMode) + { + // A 405 carrying a structured JSON-RPC error body means the peer is a Streamable HTTP server + // that rejected the method; the AutoDetect transport adopts the transport and surfaces the error + // instead of trying SSE, and neither transport should attempt initialize. + var ct = TestContext.Current.CancellationToken; + var initializeReceived = false; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + mockHttpHandler.RequestHandler = CreateStructuredProbeRejectingServer( + HttpStatusCode.MethodNotAllowed, () => initializeReceived = true); + + await using var transport = CreateTransport(httpClient, transportMode); + + await Assert.ThrowsAnyAsync(async () => + { + await using var client = await McpClient.CreateAsync(transport, new McpClientOptions(), + loggerFactory: LoggerFactory, cancellationToken: ct); + }); + + Assert.False(initializeReceived); + } + [Theory] [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.StreamableHttp)] @@ -278,8 +336,9 @@ public async Task Client_OnStructuredFallbackHttpStatusFromProbe_FallsBackTo_Ini public async Task Client_OnOtherHttpErrorFromProbe_Surfaces_NoFallback( HttpStatusCode status, HttpTransportMode transportMode) { - // Only 400, 404, and 405 indicate that the server needs the initialize handshake. Authentication - // and server failures must surface directly, without probing deprecated SSE or attempting initialize. + // Only 400 and 404 indicate that the server needs the initialize handshake (405 routes to the + // AutoDetect SSE fallback instead). Authentication and server failures must surface directly, + // without probing deprecated SSE or attempting initialize. var ct = TestContext.Current.CancellationToken; var initializeReceived = false; var sseRequested = false; diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs index c46383306..07ffb8beb 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs @@ -53,7 +53,7 @@ public async Task AutoDetectMode_UsesStreamableHttp_WhenServerSupportsIt() [Fact] public async Task AutoDetectMode_WhenBothTransportsFail_PreservesStreamableHttpException() { - // Regression test: when Streamable HTTP POST fails (e.g. 403) and the SSE GET + // Regression test: when Streamable HTTP POST fails (e.g. 404) and the SSE GET // fallback also fails (e.g. 405), the original Streamable HTTP error should // be preserved. The SSE connection failure is available as its inner exception. var options = new HttpClientTransportOptions @@ -71,11 +71,11 @@ public async Task AutoDetectMode_WhenBothTransportsFail_PreservesStreamableHttpE { if (request.Method == HttpMethod.Post) { - // Streamable HTTP POST fails with 403 (auth error) + // Streamable HTTP POST fails with 404 (an SSE-only server with no POST endpoint). return Task.FromResult(new HttpResponseMessage { - StatusCode = HttpStatusCode.Forbidden, - Content = new StringContent("Forbidden") + StatusCode = HttpStatusCode.NotFound, + Content = new StringContent("Streamable HTTP not supported") }); } @@ -99,12 +99,12 @@ public async Task AutoDetectMode_WhenBothTransportsFail_PreservesStreamableHttpE var ex = await Assert.ThrowsAsync( () => McpClient.CreateAsync(transport, cancellationToken: TestContext.Current.CancellationToken)); - Assert.Contains("403", ex.Message); + Assert.Contains("404", ex.Message); Assert.IsType(ex.InnerException); Assert.Contains("405", ex.InnerException.Message); - Assert.Equal(HttpStatusCode.Forbidden, ex.Data["ModelContextProtocol.HttpStatusCode"]); + Assert.Equal(HttpStatusCode.NotFound, ex.Data["ModelContextProtocol.HttpStatusCode"]); #if NET - Assert.Equal(HttpStatusCode.Forbidden, ex.StatusCode); + Assert.Equal(HttpStatusCode.NotFound, ex.StatusCode); #endif } @@ -278,11 +278,12 @@ await session.SendMessageAsync( } // Regression test for https://github.com/modelcontextprotocol/csharp-sdk/issues/1526 - // When Streamable HTTP returns 415 (e.g. wrong Content-Type) and the SSE fallback also fails - // (e.g. a Streamable-HTTP-only server returns 405 to the GET), the surfaced exception must - // preserve the original Streamable HTTP error rather than dropping it on the floor. + // When Streamable HTTP returns 404 (e.g. an SSE-only server with no POST endpoint) and the + // SSE fallback also fails (e.g. a Streamable-HTTP-only server returns 405 to the GET), the + // surfaced exception must preserve the original Streamable HTTP error rather than dropping + // it on the floor. [Fact] - public async Task AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturns415AndSseFallbackFails() + public async Task AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturns404AndSseFallbackFails() { var options = new HttpClientTransportOptions { @@ -295,16 +296,16 @@ public async Task AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturn using var httpClient = new HttpClient(mockHttpHandler); await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); - const string streamableHttpBody = "Content-Type must be 'application/json'"; + const string streamableHttpBody = "Streamable HTTP not supported"; mockHttpHandler.RequestHandler = (request) => { if (request.Method == HttpMethod.Post) { - // Streamable HTTP fails with 415 - this is the real server diagnostic the user needs to see. + // Streamable HTTP fails with 404 - this is the real server diagnostic the user needs to see. return Task.FromResult(new HttpResponseMessage { - StatusCode = HttpStatusCode.UnsupportedMediaType, + StatusCode = HttpStatusCode.NotFound, Content = new StringContent(streamableHttpBody), }); } @@ -312,7 +313,7 @@ public async Task AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturn if (request.Method == HttpMethod.Get) { // Streamable-HTTP-only server: SSE GET is rejected with 405. Without the fix this is the - // ONLY error the user ever sees, masking the real 415 diagnostic above. + // ONLY error the user ever sees, masking the real 404 diagnostic above. return Task.FromResult(new HttpResponseMessage { StatusCode = HttpStatusCode.MethodNotAllowed, @@ -331,11 +332,11 @@ public async Task AutoDetectMode_PreservesOriginalError_WhenStreamableHttpReturn new JsonRpcRequest { Method = RequestMethods.Initialize, Id = new RequestId(1) }, TestContext.Current.CancellationToken)); - // Walk the exception chain and assert the original 415 (and its body) is somewhere in it. + // Walk the exception chain and assert the original 404 (and its body) is somewhere in it. // We don't pin the exact exception type so this stays robust to future error-shape tweaks, // but the underlying status code and server body must reach the caller. var combined = Flatten(ex); - Assert.Contains("415", combined); + Assert.Contains("404", combined); Assert.Contains(streamableHttpBody, combined); static string Flatten(Exception e) @@ -378,7 +379,7 @@ public async Task AutoDetectMode_SurfacesStreamableHttpError_WithSseAsInner_When using var httpClient = new HttpClient(mockHttpHandler); await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); - const string streamableHttpBody = "Content-Type must be 'application/json'"; + const string streamableHttpBody = "Streamable HTTP not supported"; mockHttpHandler.RequestHandler = (request) => { @@ -386,7 +387,7 @@ public async Task AutoDetectMode_SurfacesStreamableHttpError_WithSseAsInner_When { return Task.FromResult(new HttpResponseMessage { - StatusCode = HttpStatusCode.UnsupportedMediaType, + StatusCode = HttpStatusCode.NotFound, Content = new StringContent(streamableHttpBody), }); } @@ -412,11 +413,11 @@ public async Task AutoDetectMode_SurfacesStreamableHttpError_WithSseAsInner_When // The surfaced exception is the original Streamable HTTP error (the real server diagnostic), not the SSE 405. var httpEx = Assert.IsType(ex); - Assert.Contains("415", httpEx.Message); + Assert.Contains("404", httpEx.Message); Assert.Contains(streamableHttpBody, httpEx.Message); - Assert.Equal(HttpStatusCode.UnsupportedMediaType, httpEx.Data["ModelContextProtocol.HttpStatusCode"]); + Assert.Equal(HttpStatusCode.NotFound, httpEx.Data["ModelContextProtocol.HttpStatusCode"]); #if NET - Assert.Equal(HttpStatusCode.UnsupportedMediaType, httpEx.StatusCode); + Assert.Equal(HttpStatusCode.NotFound, httpEx.StatusCode); #endif // The SSE fallback failure (the 405 from the GET) is preserved as the inner exception, not dropped. @@ -482,8 +483,8 @@ public async Task AutoDetectMode_LogsWarning_WhenSseFallbackFailsAfterStreamable { return Task.FromResult(new HttpResponseMessage { - StatusCode = HttpStatusCode.UnsupportedMediaType, - Content = new StringContent("Content-Type must be 'application/json'"), + StatusCode = HttpStatusCode.NotFound, + Content = new StringContent("Streamable HTTP not supported"), }); } From 6502de31da82ea3c78e6b3c0b14742ce3447ab3a Mon Sep 17 00:00:00 2001 From: ump45nose Date: Sun, 20 Sep 2026 03:16:57 +0000 Subject: [PATCH 3/3] fix(client): classify full AutoDetect response for 400/404/405 SSE allowlist Move ShouldSurfaceJsonRpcErrorAsProtocolException into the else-if so recognized modern errors still adopt Streamable HTTP, while parsed-but- unrecognized errors (including JSON-RPC-bodied 405) fall through to the shared HTTP classification with #1855's discover 400/404 skip and the spec allowlist. Update July/AutoDetect tests for GET/no-GET coverage, including structured non-allowlisted responses and 415. --- .../Client/July2026ProtocolFallbackTests.cs | 80 +++++++++++++++---- .../HttpClientTransportAutoDetectTests.cs | 14 ++-- 2 files changed, 73 insertions(+), 21 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs b/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs index 70efac14a..d22ac254a 100644 --- a/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/July2026ProtocolFallbackTests.cs @@ -275,8 +275,9 @@ public async Task Client_On405FromProbe_DoesNotFallBackTo_Initialize( { // 405 means the POST endpoint rejected the request method, so retrying initialize over the same // transport is not useful. The spec routes 405 to the AutoDetect transport's SSE fallback: in - // Streamable HTTP mode the 405 surfaces directly, and in AutoDetect mode the client attempts the - // deprecated SSE GET instead of initialize. Neither path may attempt initialize. + // Streamable HTTP mode the 405 surfaces directly (no GET), and in AutoDetect mode the client + // attempts the deprecated SSE GET. Assert GET/no-GET explicitly; do not treat "no initialize" + // as a general invariant after a successful SSE selection (revised #1719 will handshake then). var ct = TestContext.Current.CancellationToken; var initializeReceived = false; var sseRequested = false; @@ -295,26 +296,32 @@ await Assert.ThrowsAnyAsync(async () => loggerFactory: LoggerFactory, cancellationToken: ct); }); - Assert.False(initializeReceived); Assert.Equal(expectSseAttempt, sseRequested); + if (transportMode == HttpTransportMode.StreamableHttp) + { + // Explicit Streamable HTTP has no SSE path, so initialize must not be used to recover a 405. + Assert.False(initializeReceived); + } } [Theory] - [InlineData(HttpTransportMode.StreamableHttp)] - [InlineData(HttpTransportMode.AutoDetect)] - public async Task Client_OnStructured405FromProbe_DoesNotFallBackTo_Initialize( - HttpTransportMode transportMode) + [InlineData(HttpTransportMode.StreamableHttp, false)] + [InlineData(HttpTransportMode.AutoDetect, true)] + public async Task Client_OnStructured405FromProbe_TriesSseOnlyInAutoDetect( + HttpTransportMode transportMode, bool expectSseAttempt) { - // A 405 carrying a structured JSON-RPC error body means the peer is a Streamable HTTP server - // that rejected the method; the AutoDetect transport adopts the transport and surfaces the error - // instead of trying SSE, and neither transport should attempt initialize. + // An unrecognized JSON-RPC error on 405 is not a recognized modern signal, so AutoDetect must + // classify the full response and try SSE. Explicit Streamable HTTP surfaces the 405 with no GET. var ct = TestContext.Current.CancellationToken; var initializeReceived = false; + var sseRequested = false; using var mockHttpHandler = new MockHttpHandler(); using var httpClient = new HttpClient(mockHttpHandler); mockHttpHandler.RequestHandler = CreateStructuredProbeRejectingServer( - HttpStatusCode.MethodNotAllowed, () => initializeReceived = true); + HttpStatusCode.MethodNotAllowed, + () => initializeReceived = true, + () => sseRequested = true); await using var transport = CreateTransport(httpClient, transportMode); @@ -324,21 +331,26 @@ await Assert.ThrowsAnyAsync(async () => loggerFactory: LoggerFactory, cancellationToken: ct); }); - Assert.False(initializeReceived); + Assert.Equal(expectSseAttempt, sseRequested); + if (transportMode == HttpTransportMode.StreamableHttp) + { + Assert.False(initializeReceived); + } } [Theory] [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.UnsupportedMediaType, HttpTransportMode.StreamableHttp)] [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.AutoDetect)] [InlineData(HttpStatusCode.Unauthorized, HttpTransportMode.AutoDetect)] [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.UnsupportedMediaType, HttpTransportMode.AutoDetect)] public async Task Client_OnOtherHttpErrorFromProbe_Surfaces_NoFallback( HttpStatusCode status, HttpTransportMode transportMode) { - // Only 400 and 404 indicate that the server needs the initialize handshake (405 routes to the - // AutoDetect SSE fallback instead). Authentication and server failures must surface directly, - // without probing deprecated SSE or attempting initialize. + // Non-allowlisted statuses (401/403/5xx/415/…) must surface directly for both structured and + // unstructured bodies — no initialize handshake and no deprecated SSE GET. var ct = TestContext.Current.CancellationToken; var initializeReceived = false; var sseRequested = false; @@ -360,6 +372,39 @@ await Assert.ThrowsAnyAsync(async () => Assert.False(sseRequested); } + [Theory] + [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.UnsupportedMediaType, HttpTransportMode.StreamableHttp)] + [InlineData(HttpStatusCode.InternalServerError, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.Unauthorized, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.Forbidden, HttpTransportMode.AutoDetect)] + [InlineData(HttpStatusCode.UnsupportedMediaType, HttpTransportMode.AutoDetect)] + public async Task Client_OnStructuredOtherHttpErrorFromProbe_Surfaces_NoGet( + HttpStatusCode status, HttpTransportMode transportMode) + { + // Same non-allowlist coverage for unrecognized JSON-RPC-bodied responses: no SSE GET. + var ct = TestContext.Current.CancellationToken; + var initializeReceived = false; + var sseRequested = false; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + mockHttpHandler.RequestHandler = CreateStructuredProbeRejectingServer( + status, () => initializeReceived = true, () => sseRequested = true); + + await using var transport = CreateTransport(httpClient, transportMode); + + await Assert.ThrowsAnyAsync(async () => + { + await using var client = await McpClient.CreateAsync(transport, new McpClientOptions(), + loggerFactory: LoggerFactory, cancellationToken: ct); + }); + + Assert.False(initializeReceived); + Assert.False(sseRequested); + } + private HttpClientTransport CreateTransport(HttpClient httpClient, HttpTransportMode transportMode) => new(new HttpClientTransportOptions { @@ -414,11 +459,14 @@ private static Func> CreateProbeRe }; private static Func> CreateStructuredProbeRejectingServer( - HttpStatusCode probeStatus, Action onInitialize) + HttpStatusCode probeStatus, Action onInitialize, Action? onSseRequest = null) => async request => { if (request.Method == HttpMethod.Get) + { + onSseRequest?.Invoke(); return EmptyResponse(HttpStatusCode.MethodNotAllowed); + } var body = await request.Content!.ReadAsStringAsync(); using var doc = JsonDocument.Parse(body); diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs index 07ffb8beb..8823d4c35 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs @@ -617,10 +617,10 @@ await session.SendMessageAsync( await ssePipe.Writer.CompleteAsync(); } - // The skip is scoped to the two statuses ConnectAsync acts on. Any other failure on the discover probe - // keeps the original fallback, because it is not evidence that an initialize retry is coming. + // 415 is outside the spec's 400/404/405 SSE-fallback allowlist, so AutoDetect must surface it + // without a deprecated GET — including when the failing request happens to be server/discover. [Fact] - public async Task AutoDetectMode_FallsBackToSse_WhenDiscoverProbeFailsWithUnrelatedStatus() + public async Task AutoDetectMode_SkipsSseFallback_WhenDiscoverProbeFailsWithNonAllowlistedStatus() { var options = new HttpClientTransportOptions { @@ -650,11 +650,15 @@ public async Task AutoDetectMode_FallsBackToSse_WhenDiscoverProbeFailsWithUnrela await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken); - await Assert.ThrowsAsync(() => + var ex = await Assert.ThrowsAsync(() => session.SendMessageAsync( new JsonRpcRequest { Method = RequestMethods.ServerDiscover, Id = new RequestId(1) }, TestContext.Current.CancellationToken)); - Assert.Equal(1, getCount); + Assert.Equal(0, getCount); + Assert.Equal(HttpStatusCode.UnsupportedMediaType, ex.Data["ModelContextProtocol.HttpStatusCode"]); + Assert.DoesNotContain( + MockLoggerProvider.LogMessages, + m => m.Message.Contains("falling back to SSE transport")); } }