Echo request id in post-parse Streamable HTTP error responses (#1687)
Co-authored-by: Tarek Mahmoud Sayed <tarekms@ntdev.microsoft.com>
This commit is contained in:
parent
5d00eedebb
commit
e23be1d75e
3 changed files with 82 additions and 19 deletions
|
|
@ -106,13 +106,18 @@ internal sealed class StreamableHttpHandler(
|
|||
return;
|
||||
}
|
||||
|
||||
// Once the body has been parsed into a request with a readable id, every JSON-RPC error response
|
||||
// for this request MUST echo that id (base protocol responses section; SEP-2243 error format).
|
||||
// Notifications carry no id, so this stays default (null), which is correct.
|
||||
var requestId = message is JsonRpcRequest jsonRpcRequest ? jsonRpcRequest.Id : default;
|
||||
|
||||
if (!ValidateMcpHeaders(context, message, mcpServerOptionsSnapshot.Value.ToolCollection, out var errorMessage))
|
||||
{
|
||||
await WriteJsonRpcErrorAsync(context, errorMessage, StatusCodes.Status400BadRequest, (int)McpErrorCode.HeaderMismatch);
|
||||
await WriteJsonRpcErrorAsync(context, errorMessage, StatusCodes.Status400BadRequest, (int)McpErrorCode.HeaderMismatch, requestId);
|
||||
return;
|
||||
}
|
||||
|
||||
var session = await GetOrCreateSessionAsync(context, message);
|
||||
var session = await GetOrCreateSessionAsync(context, message, requestId);
|
||||
if (session is null)
|
||||
{
|
||||
return;
|
||||
|
|
@ -286,7 +291,7 @@ internal sealed class StreamableHttpHandler(
|
|||
}
|
||||
}
|
||||
|
||||
private async ValueTask<StreamableHttpSession?> GetSessionAsync(HttpContext context, string sessionId)
|
||||
private async ValueTask<StreamableHttpSession?> GetSessionAsync(HttpContext context, string sessionId, RequestId requestId = default)
|
||||
{
|
||||
if (string.IsNullOrEmpty(sessionId))
|
||||
{
|
||||
|
|
@ -294,7 +299,7 @@ internal sealed class StreamableHttpHandler(
|
|||
"Bad Request: Mcp-Session-Id header is required for GET and DELETE requests when the server is using sessions. " +
|
||||
"If your server doesn't need sessions, enable stateless mode by setting HttpServerTransportOptions.Stateless = true. " +
|
||||
"See https://csharp.sdk.modelcontextprotocol.io/concepts/stateless/stateless.html for more details.",
|
||||
StatusCodes.Status400BadRequest);
|
||||
StatusCodes.Status400BadRequest, requestId: requestId);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -309,7 +314,7 @@ internal sealed class StreamableHttpHandler(
|
|||
// One of the few other usages I found was from some Ethereum JSON-RPC documentation and this
|
||||
// JSON-RPC library from Microsoft called StreamJsonRpc where it's called JsonRpcErrorCode.NoMarshaledObjectFound
|
||||
// https://learn.microsoft.com/dotnet/api/streamjsonrpc.protocol.jsonrpcerrorcode?view=streamjsonrpc-2.9#fields
|
||||
await WriteJsonRpcErrorAsync(context, "Session not found", StatusCodes.Status404NotFound, -32001);
|
||||
await WriteJsonRpcErrorAsync(context, "Session not found", StatusCodes.Status404NotFound, -32001, requestId);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
|
@ -318,7 +323,7 @@ internal sealed class StreamableHttpHandler(
|
|||
{
|
||||
await WriteJsonRpcErrorAsync(context,
|
||||
"Forbidden: The currently authenticated user does not match the user who initiated the session.",
|
||||
StatusCodes.Status403Forbidden);
|
||||
StatusCodes.Status403Forbidden, requestId: requestId);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -367,7 +372,7 @@ internal sealed class StreamableHttpHandler(
|
|||
}
|
||||
}
|
||||
|
||||
private async ValueTask<StreamableHttpSession?> GetOrCreateSessionAsync(HttpContext context, JsonRpcMessage message)
|
||||
private async ValueTask<StreamableHttpSession?> GetOrCreateSessionAsync(HttpContext context, JsonRpcMessage message, RequestId requestId = default)
|
||||
{
|
||||
var sessionId = context.Request.Headers[McpSessionIdHeaderName].ToString();
|
||||
|
||||
|
|
@ -380,7 +385,7 @@ internal sealed class StreamableHttpHandler(
|
|||
// A request carrying an Mcp-Session-Id is non-conformant under the 2026-07-28 revision (SEP-2567).
|
||||
await WriteJsonRpcErrorAsync(context,
|
||||
"Bad Request: Mcp-Session-Id is not supported by the 2026-07-28 and later protocol revisions (SEP-2567).",
|
||||
StatusCodes.Status400BadRequest);
|
||||
StatusCodes.Status400BadRequest, requestId: requestId);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -389,7 +394,7 @@ internal sealed class StreamableHttpHandler(
|
|||
// The author explicitly opted into sessions (Stateless = false), which the 2026-07-28
|
||||
// revision cannot provide. Refuse it so a dual-era client falls back to the legacy
|
||||
// initialize handshake and gets the session it asked for (SEP-2575 fallback semantics).
|
||||
await WriteUnsupportedProtocolVersionErrorAsync(context);
|
||||
await WriteUnsupportedProtocolVersionErrorAsync(context, requestId);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -408,7 +413,7 @@ internal sealed class StreamableHttpHandler(
|
|||
"Bad Request: A new session can only be created by an initialize request. Include a valid Mcp-Session-Id header for non-initialize requests, " +
|
||||
"or enable stateless mode by setting HttpServerTransportOptions.Stateless = true if your server doesn't need sessions. " +
|
||||
"See https://csharp.sdk.modelcontextprotocol.io/concepts/stateless/stateless.html for more details.",
|
||||
StatusCodes.Status400BadRequest);
|
||||
StatusCodes.Status400BadRequest, requestId: requestId);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -418,12 +423,12 @@ internal sealed class StreamableHttpHandler(
|
|||
{
|
||||
// In stateless mode, we should not be getting existing sessions via sessionId
|
||||
// This path should not be reached in stateless mode
|
||||
await WriteJsonRpcErrorAsync(context, "Bad Request: The Mcp-Session-Id header is not supported in stateless mode", StatusCodes.Status400BadRequest);
|
||||
await WriteJsonRpcErrorAsync(context, "Bad Request: The Mcp-Session-Id header is not supported in stateless mode", StatusCodes.Status400BadRequest, requestId: requestId);
|
||||
return null;
|
||||
}
|
||||
else
|
||||
{
|
||||
return await GetSessionAsync(context, sessionId);
|
||||
return await GetSessionAsync(context, sessionId, requestId);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -568,10 +573,11 @@ internal sealed class StreamableHttpHandler(
|
|||
return eventStreamReader;
|
||||
}
|
||||
|
||||
private static Task WriteJsonRpcErrorAsync(HttpContext context, string errorMessage, int statusCode, int errorCode = -32000)
|
||||
private static Task WriteJsonRpcErrorAsync(HttpContext context, string errorMessage, int statusCode, int errorCode = -32000, RequestId requestId = default)
|
||||
{
|
||||
var jsonRpcError = new JsonRpcError
|
||||
{
|
||||
Id = requestId,
|
||||
Error = new()
|
||||
{
|
||||
Code = errorCode,
|
||||
|
|
@ -691,7 +697,7 @@ internal sealed class StreamableHttpHandler(
|
|||
/// that excludes 2026-07-28 and later. A dual-era client then falls back to the legacy initialize handshake
|
||||
/// (SEP-2575).
|
||||
/// </summary>
|
||||
private static Task WriteUnsupportedProtocolVersionErrorAsync(HttpContext context)
|
||||
private static Task WriteUnsupportedProtocolVersionErrorAsync(HttpContext context, RequestId requestId = default)
|
||||
{
|
||||
var requestedProtocolVersion = context.Request.Headers[McpProtocolVersionHeaderName].ToString();
|
||||
var errorDetail = new JsonRpcErrorDetail
|
||||
|
|
@ -708,7 +714,7 @@ internal sealed class StreamableHttpHandler(
|
|||
GetRequiredJsonTypeInfo<UnsupportedProtocolVersionErrorData>()),
|
||||
};
|
||||
|
||||
return WriteJsonRpcErrorDetailAsync(context, errorDetail, StatusCodes.Status400BadRequest);
|
||||
return WriteJsonRpcErrorDetailAsync(context, errorDetail, StatusCodes.Status400BadRequest, requestId);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -1150,9 +1156,9 @@ internal sealed class StreamableHttpHandler(
|
|||
return SafeIntegerParse.NotNumeric;
|
||||
}
|
||||
|
||||
private static Task WriteJsonRpcErrorDetailAsync(HttpContext context, JsonRpcErrorDetail detail, int statusCode)
|
||||
private static Task WriteJsonRpcErrorDetailAsync(HttpContext context, JsonRpcErrorDetail detail, int statusCode, RequestId requestId = default)
|
||||
{
|
||||
var jsonRpcError = new JsonRpcError { Error = detail };
|
||||
var jsonRpcError = new JsonRpcError { Id = requestId, Error = detail };
|
||||
return Results.Json(jsonRpcError, s_errorTypeInfo, statusCode: statusCode).ExecuteAsync(context);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -174,7 +174,7 @@ public class RawHttpConformanceTests(ITestOutputHelper outputHelper) : KestrelIn
|
|||
// checks) so a conformant client on this revision surfaces the error instead of mistaking the modern server for a
|
||||
// legacy one and falling back to the initialize handshake.
|
||||
var body =
|
||||
@"{""jsonrpc"":""2.0"",""id"":1,""method"":""server/discover"",""params"":{" +
|
||||
@"{""jsonrpc"":""2.0"",""id"":4242,""method"":""server/discover"",""params"":{" +
|
||||
July2026ProtocolMetaFragment("2025-11-25") + "}}";
|
||||
|
||||
using var request = new HttpRequestMessage(HttpMethod.Post, "") { Content = JsonContent(body) };
|
||||
|
|
@ -184,6 +184,33 @@ public class RawHttpConformanceTests(ITestOutputHelper outputHelper) : KestrelIn
|
|||
|
||||
var json = await ReadJsonResponseAsync(response, TestContext.Current.CancellationToken);
|
||||
Assert.Equal((int)McpErrorCode.HeaderMismatch, json["error"]!["code"]!.GetValue<int>());
|
||||
|
||||
// The body parsed successfully, so per the base protocol responses section (and SEP-2243's error
|
||||
// response format) this error MUST echo the request id rather than emitting id=null (see #1677).
|
||||
Assert.Equal(4242, json["id"]!.GetValue<long>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task July2026Post_MissingMcpNameHeader_ReturnsHeaderMismatch_EchoesRequestId()
|
||||
{
|
||||
await StartAsync();
|
||||
|
||||
// A well-formed tools/call whose body parses, but the required Mcp-Name header is absent. The server
|
||||
// rejects it with -32020 HeaderMismatch, and because the request id was readable the JSON-RPC error
|
||||
// MUST carry that same id (regression guard for #1677).
|
||||
var body =
|
||||
@"{""jsonrpc"":""2.0"",""id"":4242,""method"":""tools/call"",""params"":{""name"":""echo"",""arguments"":{""text"":""hi""}," +
|
||||
July2026ProtocolMetaFragment() + "}}";
|
||||
|
||||
using var request = new HttpRequestMessage(HttpMethod.Post, "") { Content = JsonContent(body) };
|
||||
request.Headers.Add(ProtocolVersionHeader, McpHttpHeaders.July2026ProtocolVersion);
|
||||
request.Headers.Add("Mcp-Method", "tools/call");
|
||||
using var response = await HttpClient.SendAsync(request, TestContext.Current.CancellationToken);
|
||||
|
||||
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||
var json = await ReadJsonResponseAsync(response, TestContext.Current.CancellationToken);
|
||||
Assert.Equal((int)McpErrorCode.HeaderMismatch, json["error"]!["code"]!.GetValue<int>());
|
||||
Assert.Equal(4242, json["id"]!.GetValue<long>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
|
|||
|
|
@ -224,7 +224,7 @@ public class StreamableHttpServerConformanceTests(ITestOutputHelper outputHelper
|
|||
|
||||
using var request = new HttpRequestMessage(HttpMethod.Post, "")
|
||||
{
|
||||
Content = JsonContent(EchoRequest),
|
||||
Content = JsonContent("""{"jsonrpc":"2.0","id":4242,"method":"tools/call","params":{"name":"echo","arguments":{"message":"hi"}}}"""),
|
||||
Headers =
|
||||
{
|
||||
{ "mcp-session-id", "fakeSession" },
|
||||
|
|
@ -232,6 +232,11 @@ public class StreamableHttpServerConformanceTests(ITestOutputHelper outputHelper
|
|||
};
|
||||
using var response = await HttpClient.SendAsync(request, TestContext.Current.CancellationToken);
|
||||
Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
|
||||
|
||||
// The request body parsed successfully, so the JSON-RPC error MUST echo its id rather than
|
||||
// emitting id=null (base protocol responses section; see #1677).
|
||||
using var doc = JsonDocument.Parse(await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken));
|
||||
Assert.Equal(4242, doc.RootElement.GetProperty("id").GetInt64());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
@ -245,6 +250,31 @@ public class StreamableHttpServerConformanceTests(ITestOutputHelper outputHelper
|
|||
var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken);
|
||||
Assert.Contains("Mcp-Session-Id", body);
|
||||
Assert.Contains("Stateless", body);
|
||||
|
||||
// The request body parsed successfully, so the JSON-RPC error MUST echo its id (see #1677).
|
||||
using var doc = JsonDocument.Parse(body);
|
||||
Assert.Equal(1, doc.RootElement.GetProperty("id").GetInt64());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task StatelessPostWithSessionId_Returns400_EchoesRequestId()
|
||||
{
|
||||
await StartAsync(stateless: true);
|
||||
|
||||
using var request = new HttpRequestMessage(HttpMethod.Post, "")
|
||||
{
|
||||
Content = JsonContent("""{"jsonrpc":"2.0","id":4242,"method":"tools/list","params":{}}"""),
|
||||
Headers =
|
||||
{
|
||||
{ "mcp-session-id", "someSession" },
|
||||
},
|
||||
};
|
||||
using var response = await HttpClient.SendAsync(request, TestContext.Current.CancellationToken);
|
||||
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||
|
||||
// The request body parsed successfully, so the stateless-mode rejection MUST echo its id (see #1677).
|
||||
using var doc = JsonDocument.Parse(await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken));
|
||||
Assert.Equal(4242, doc.RootElement.GetProperty("id").GetInt64());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
|
|||
Loading…
Reference in a new issue