From 547356e25c265f32bc983ee73492d4a6a8c22a97 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 5 Jun 2026 22:05:41 -0700 Subject: [PATCH 1/4] Reply with JSON-RPC parse error for over-nested stdio requests When a stdio JSON-RPC request arrived with params nested deeper than the JSON reader's default MaxDepth of 64, full deserialization threw a JsonException that the read loop logged and swallowed. The request id was never recovered, so no response reached the client and the id-bearing request stayed pending until the caller timed out, even though the server process kept handling later requests. The read loop now recovers the request id from the raw line when full parsing fails and replies with a JSON-RPC parse error for that id, so the caller's pending request completes promptly. Id recovery walks only the top-level object with an elevated reader depth and skips other values, so a deeply nested params value cannot make recovery itself fail. Requests without a recoverable id keep the previous behavior of logging and continuing. Fixes #1614 --- .../Server/StreamServerTransport.cs | 103 +++++++++++++++++- .../Transport/StdioServerTransportTests.cs | 45 ++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs b/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs index 5e1a106c5..194513822 100644 --- a/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs +++ b/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs @@ -137,7 +137,32 @@ private async Task ReadMessagesAsync() LogTransportMessageParseFailed(Name, ex); } - // Continue reading even if we fail to parse a message + // Deserializing the full message failed, for example because the params object was nested + // more deeply than the JSON reader's MaxDepth allows. If the message still carried a request + // id, reply with a JSON-RPC parse error using that id so the caller's pending request + // completes instead of hanging until it times out. If no id can be recovered, the message + // was either a notification or too malformed to correlate, so we just continue reading. + if (TryRecoverRequestId(line, out RequestId id)) + { + var errorResponse = new JsonRpcError + { + Id = id, + Error = new JsonRpcErrorDetail + { + Code = (int)McpErrorCode.ParseError, + Message = "Failed to parse the JSON-RPC request.", + }, + }; + + try + { + await SendMessageAsync(errorResponse, shutdownToken).ConfigureAwait(false); + } + catch (Exception sendEx) when (sendEx is not OperationCanceledException) + { + LogTransportSendFailed(Name, id.ToString(), sendEx); + } + } } } } @@ -156,6 +181,82 @@ private async Task ReadMessagesAsync() } } + /// + /// Attempts to recover the JSON-RPC request id from a line that failed full deserialization. + /// + /// + /// This walks only the top-level object looking for an "id" property and skips every other value, + /// using a large reader depth so a deeply nested "params" value cannot make recovery itself fail. + /// + private static bool TryRecoverRequestId(string line, out RequestId id) + { + id = default; + + try + { + byte[] utf8 = Encoding.UTF8.GetBytes(line); + var reader = new Utf8JsonReader(utf8, new JsonReaderOptions + { + // Use the maximum reader depth so that an over-nested "params" value cannot make id + // recovery throw for the same reason the original parse did. + MaxDepth = int.MaxValue, + }); + + if (!reader.Read() || reader.TokenType != JsonTokenType.StartObject) + { + return false; + } + + while (reader.Read()) + { + if (reader.TokenType == JsonTokenType.EndObject) + { + break; + } + + if (reader.TokenType != JsonTokenType.PropertyName) + { + continue; + } + + bool isId = reader.ValueTextEquals("id"u8); + + if (!reader.Read()) + { + break; + } + + if (isId) + { + switch (reader.TokenType) + { + case JsonTokenType.String: + id = new RequestId(reader.GetString()!); + return true; + + case JsonTokenType.Number when reader.TryGetInt64(out long longId): + id = new RequestId(longId); + return true; + + default: + // An id that is neither a string nor an integer cannot be correlated, so + // there is no point sending an error response for it. + return false; + } + } + + // Skip the value of any property other than id, including a deeply nested params object. + reader.Skip(); + } + } + catch (JsonException) + { + // The line was too malformed to even locate a top-level id; nothing to correlate. + } + + return false; + } + /// public override async ValueTask DisposeAsync() { diff --git a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs index e47269686..fe5db846c 100644 --- a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs @@ -278,6 +278,51 @@ public async Task ReadMessagesAsync_Should_Accept_CRLF_Delimited_Messages() Assert.Equal("44", ((JsonRpcRequest)readMessage).Id.ToString()); } + [Fact] + public async Task ReadMessagesAsync_Should_Respond_With_ParseError_For_Request_Exceeding_MaxDepth() + { + // Build a ping request whose params nest more deeply than System.Text.Json's default + // reader MaxDepth of 64, which is what makes full deserialization throw. The request still + // carries an id, so the transport should reply with a JSON-RPC parse error for that id rather + // than dropping the request and leaving the caller pending. + var nested = new StringBuilder(); + const int depth = 100; + for (int i = 0; i < depth; i++) + { + nested.Append("{\"p").Append(i).Append("\":"); + } + nested.Append("{\"leaf\":true}"); + nested.Append('}', depth); + + var requestLine = $"{{\"jsonrpc\":\"2.0\",\"id\":900100,\"method\":\"ping\",\"params\":{nested}}}"; + + Pipe inputPipe = new(); + Pipe outputPipe = new(); + using var input = inputPipe.Reader.AsStream(); + using var output = outputPipe.Writer.AsStream(); + + await using var transport = new StreamServerTransport( + input, + output, + loggerFactory: LoggerFactory); + + await inputPipe.Writer.WriteAsync(Encoding.UTF8.GetBytes($"{requestLine}\n"), TestContext.Current.CancellationToken); + + // Read the single response line the transport writes back to the output stream. + using var responseReader = new StreamReader(outputPipe.Reader.AsStream(), Encoding.UTF8); + var responseLine = await responseReader.ReadLineAsync(TestContext.Current.CancellationToken); + + Assert.NotNull(responseLine); + + var response = JsonSerializer.Deserialize(responseLine!, McpJsonUtilities.DefaultOptions); + var error = Assert.IsType(response); + Assert.Equal("900100", error.Id.ToString()); + Assert.Equal((int)McpErrorCode.ParseError, error.Error.Code); + + // The transport should still be reading further messages after recovering from the bad one. + Assert.True(transport.IsConnected); + } + [Fact] public async Task ReadMessagesAsync_Should_Log_Received_At_Trace_Level() { From 56af319bfafc8d6d579463891f1d5e508712f27c Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 5 Jun 2026 22:12:21 -0700 Subject: [PATCH 2/4] Guard ReadLineAsync cancellation token for net472 in test The net472 target lacks the TextReader.ReadLineAsync(CancellationToken) overload, so the new test failed to compile on the Windows build legs. Wrap the token in an #if NET directive to match the existing pattern in SseResponseStreamTransportTests, falling back to the parameterless overload on .NET Framework. --- .../Transport/StdioServerTransportTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs index fe5db846c..42472556d 100644 --- a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs @@ -310,7 +310,11 @@ public async Task ReadMessagesAsync_Should_Respond_With_ParseError_For_Request_E // Read the single response line the transport writes back to the output stream. using var responseReader = new StreamReader(outputPipe.Reader.AsStream(), Encoding.UTF8); - var responseLine = await responseReader.ReadLineAsync(TestContext.Current.CancellationToken); + var responseLine = await responseReader.ReadLineAsync( +#if NET + TestContext.Current.CancellationToken +#endif + ); Assert.NotNull(responseLine); From fe9bc89542edd46c0f79e4439bfeaa1d152fc58e Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 5 Jun 2026 22:53:08 -0700 Subject: [PATCH 3/4] Retrigger CI, filter-order test flaked on net10 despite the 1627 fix From 7a28d2bf25471cde6d4d470bb082977f5ee90ea8 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:56:43 -0700 Subject: [PATCH 4/4] Harden stdio request id recovery against non-JSON exceptions TryRecoverRequestId caught only JsonException. Any other exception from the best-effort id recovery would propagate out of the read loop and disconnect the transport, turning a single bad message into a full session teardown. Broaden the catch so recovery never escalates a bad message into a session teardown. --- .../Server/StreamServerTransport.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs b/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs index 194513822..c8c5d8408 100644 --- a/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs +++ b/src/ModelContextProtocol.Core/Server/StreamServerTransport.cs @@ -160,7 +160,8 @@ private async Task ReadMessagesAsync() } catch (Exception sendEx) when (sendEx is not OperationCanceledException) { - LogTransportSendFailed(Name, id.ToString(), sendEx); + // Swallow so a failed error-send does not tear down the read loop. No logging + // here because SendMessageAsync already logs send failures before it throws. } } } @@ -249,9 +250,12 @@ private static bool TryRecoverRequestId(string line, out RequestId id) reader.Skip(); } } - catch (JsonException) + catch (Exception) { - // The line was too malformed to even locate a top-level id; nothing to correlate. + // Recovery is best effort. Whether the line was too malformed to locate a top-level id or + // the reader failed for any other reason, swallow it here and return false. Letting an + // exception escape would propagate out of the read loop and disconnect the transport, + // turning a single bad message into a full session teardown. } return false;