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..42472556d 100644 --- a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs @@ -278,6 +278,55 @@ 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( +#if NET + TestContext.Current.CancellationToken +#endif + ); + + 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() {