Reply with JSON-RPC parse error for over-nested stdio requests (#1629)
Co-authored-by: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com>
This commit is contained in:
parent
ba372faece
commit
990bd76b53
1 changed files with 155 additions and 1 deletions
|
|
@ -137,7 +137,33 @@ public class StreamServerTransport : TransportBase
|
|||
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)
|
||||
{
|
||||
// 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.
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -156,6 +182,85 @@ public class StreamServerTransport : TransportBase
|
|||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Attempts to recover the JSON-RPC request id from a line that failed full deserialization.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// 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.
|
||||
/// </remarks>
|
||||
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 (Exception)
|
||||
{
|
||||
// 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;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public override async ValueTask DisposeAsync()
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue