Fix regression matching templated McpServerResources (#897)

This commit is contained in:
Stephen Halter 2025-10-20 13:13:37 -05:00 • committed by GitHub
commit 1343dd3ebd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 107 additions and 46 deletions

View file

@ -23,11 +23,12 @@ public class HttpServerTransportOptions
public Func<HttpContext, McpServer, CancellationToken, Task>? RunSessionHandler { get; set; } public Func<HttpContext, McpServer, CancellationToken, Task>? RunSessionHandler { get; set; }
/// <summary> /// <summary>
/// Gets or sets whether the server should run in a stateless mode which allows for load balancing without session affinity. /// Gets or sets whether the server should run in a stateless mode which does not track state between requests
/// allowing for load balancing without session affinity.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// If <see langword="true"/>, <see cref="RunSessionHandler"/> is called once for every request for each request, /// If <see langword="true"/>, <see cref="McpSession.SessionId"/> will be null, and the "MCP-Session-Id" header will not be used,
/// the "/sse" endpoint will be disabled, and the "MCP-Session-Id" header will not be used. /// the <see cref="RunSessionHandler"/> will be called once for for each request, and the "/sse" endpoint will be disabled.
/// Unsolicited server-to-client messages and all server-to-client requests are also unsupported, because any responses /// Unsolicited server-to-client messages and all server-to-client requests are also unsupported, because any responses
/// may arrive at another ASP.NET Core application process. /// may arrive at another ASP.NET Core application process.
/// Client sampling and roots capabilities are also disabled in stateless mode, because the server cannot make requests. /// Client sampling and roots capabilities are also disabled in stateless mode, because the server cannot make requests.

View file

@ -12,7 +12,7 @@ namespace ModelContextProtocol;
/// ///
/// This exception type can be thrown by MCP tools or tool call filters to propagate detailed error messages /// This exception type can be thrown by MCP tools or tool call filters to propagate detailed error messages
/// from <see cref="Exception.Message"/> when a tool execution fails via a <see cref="CallToolResult"/>. /// from <see cref="Exception.Message"/> when a tool execution fails via a <see cref="CallToolResult"/>.
/// For non-tool calls, this exception controls the message propogated via a <see cref="JsonRpcError"/>. /// For non-tool calls, this exception controls the message propagated via a <see cref="JsonRpcError"/>.
/// ///
/// <see cref="McpProtocolException"/> is a derived type that can be used to also specify the /// <see cref="McpProtocolException"/> is a derived type that can be used to also specify the
/// <see cref="McpErrorCode"/> that should be used for the resulting <see cref="JsonRpcError"/>. /// <see cref="McpErrorCode"/> that should be used for the resulting <see cref="JsonRpcError"/>.

View file

@ -314,7 +314,28 @@ internal sealed class AIFunctionMcpServerResource : McpServerResource
public override IReadOnlyList<object> Metadata => _metadata; public override IReadOnlyList<object> Metadata => _metadata;
/// <inheritdoc /> /// <inheritdoc />
public override async ValueTask<ReadResourceResult?> ReadAsync( public override bool IsMatch(string uri)
{
Throw.IfNull(uri);
return TryMatch(uri, out _);
}
private bool TryMatch(string uri, out Match? match)
{
// For templates, use the Regex to parse. For static resources, we can just compare the URIs.
if (_uriParser is null)
{
// This resource is not templated.
match = null;
return UriTemplate.UriTemplateComparer.Instance.Equals(uri, ProtocolResourceTemplate.UriTemplate);
}
match = _uriParser.Match(uri);
return match.Success;
}
/// <inheritdoc />
public override async ValueTask<ReadResourceResult> ReadAsync(
RequestContext<ReadResourceRequestParams> request, CancellationToken cancellationToken = default) RequestContext<ReadResourceRequestParams> request, CancellationToken cancellationToken = default)
{ {
Throw.IfNull(request); Throw.IfNull(request);
@ -323,20 +344,9 @@ internal sealed class AIFunctionMcpServerResource : McpServerResource
cancellationToken.ThrowIfCancellationRequested(); cancellationToken.ThrowIfCancellationRequested();
// Check to see if this URI template matches the request URI. If it doesn't, return null. if (!TryMatch(request.Params.Uri, out Match? match))
// For templates, use the Regex to parse. For static resources, we can just compare the URIs.
Match? match = null;
if (_uriParser is not null)
{ {
match = _uriParser.Match(request.Params.Uri); throw new InvalidOperationException($"Resource '{ProtocolResourceTemplate.UriTemplate}' does not match the provided URI '{request.Params.Uri}'.");
if (!match.Success)
{
return null;
}
}
else if (!UriTemplate.UriTemplateComparer.Instance.Equals(request.Params.Uri, ProtocolResource!.Uri))
{
return null;
} }
// Build up the arguments for the AIFunction call, including all of the name/value pairs from the URI. // Build up the arguments for the AIFunction call, including all of the name/value pairs from the URI.

View file

@ -26,7 +26,10 @@ public abstract class DelegatingMcpServerResource : McpServerResource
public override ResourceTemplate ProtocolResourceTemplate => _innerResource.ProtocolResourceTemplate; public override ResourceTemplate ProtocolResourceTemplate => _innerResource.ProtocolResourceTemplate;
/// <inheritdoc /> /// <inheritdoc />
public override ValueTask<ReadResourceResult?> ReadAsync(RequestContext<ReadResourceRequestParams> request, CancellationToken cancellationToken = default) => public override bool IsMatch(string uri) => _innerResource.IsMatch(uri);
/// <inheritdoc />
public override ValueTask<ReadResourceResult> ReadAsync(RequestContext<ReadResourceRequestParams> request, CancellationToken cancellationToken = default) =>
_innerResource.ReadAsync(request, cancellationToken); _innerResource.ReadAsync(request, cancellationToken);
/// <inheritdoc /> /// <inheritdoc />

View file

@ -342,10 +342,7 @@ internal sealed partial class McpServerImpl : McpServer
{ {
if (request.MatchedPrimitive is McpServerResource matchedResource) if (request.MatchedPrimitive is McpServerResource matchedResource)
{ {
if (await matchedResource.ReadAsync(request, cancellationToken).ConfigureAwait(false) is { } result) return await matchedResource.ReadAsync(request, cancellationToken).ConfigureAwait(false);
{
return result;
}
} }
return await originalReadResourceHandler(request, cancellationToken).ConfigureAwait(false); return await originalReadResourceHandler(request, cancellationToken).ConfigureAwait(false);
@ -366,22 +363,17 @@ internal sealed partial class McpServerImpl : McpServer
if (request.Params?.Uri is { } uri && resources is not null) if (request.Params?.Uri is { } uri && resources is not null)
{ {
// First try an O(1) lookup by exact match. // First try an O(1) lookup by exact match.
if (resources.TryGetPrimitive(uri, out var resource)) if (resources.TryGetPrimitive(uri, out var resource) && !resource.IsTemplated)
{ {
request.MatchedPrimitive = resource; request.MatchedPrimitive = resource;
} }
else else
{ {
// Fall back to an O(N) lookup, trying to match against each URI template. // Fall back to an O(N) lookup, trying to match against each URI template.
// The number of templates is controlled by the server developer, and the number is expected to be
// not terribly large. If that changes, this can be tweaked to enable a more efficient lookup.
foreach (var resourceTemplate in resources) foreach (var resourceTemplate in resources)
{ {
// Check if this template would handle the request by testing if ReadAsync would succeed if (resourceTemplate.IsMatch(uri))
if (resourceTemplate.IsTemplated)
{ {
// This is a simplified check - a more robust implementation would match the URI pattern
// For now, we'll let the actual handler attempt the match
request.MatchedPrimitive = resourceTemplate; request.MatchedPrimitive = resourceTemplate;
break; break;
} }

View file

@ -162,6 +162,16 @@ public abstract class McpServerResource : IMcpServerPrimitive
/// </remarks> /// </remarks>
public abstract IReadOnlyList<object> Metadata { get; } public abstract IReadOnlyList<object> Metadata { get; }
/// <summary>
/// Evaluates whether the <paramref name="uri"/> matches the <see cref="ProtocolResourceTemplate"/>
/// and can be used as the <see cref="ReadResourceRequestParams.Uri"/> passed to <see cref="ReadAsync"/>.
/// </summary>
/// <param name="uri">The URI being evaluated for this resource.</param>
/// <returns>
/// <see langword="true"/> if the <paramref name="uri"/> matches the <see cref="ProtocolResourceTemplate"/>; otherwise, <see langword="false"/>.
/// </returns>
public abstract bool IsMatch(string uri);
/// <summary> /// <summary>
/// Gets the resource, rendering it with the provided request parameters and returning the resource result. /// Gets the resource, rendering it with the provided request parameters and returning the resource result.
/// </summary> /// </summary>
@ -174,12 +184,14 @@ public abstract class McpServerResource : IMcpServerPrimitive
/// </param> /// </param>
/// <returns> /// <returns>
/// A <see cref="ValueTask{ReadResourceResult}"/> representing the asynchronous operation, containing a <see cref="ReadResourceResult"/> with /// A <see cref="ValueTask{ReadResourceResult}"/> representing the asynchronous operation, containing a <see cref="ReadResourceResult"/> with
/// the resource content and messages. If and only if this <see cref="McpServerResource"/> doesn't match the <see cref="ReadResourceRequestParams.Uri"/>, /// the resource content and messages.
/// the method returns <see langword="null"/>.
/// </returns> /// </returns>
/// <exception cref="ArgumentNullException"><paramref name="request"/> is <see langword="null"/>.</exception> /// <exception cref="ArgumentNullException"><paramref name="request"/> is <see langword="null"/>.</exception>
/// <exception cref="InvalidOperationException">The resource implementation returned <see langword="null"/> or an unsupported result type.</exception> /// <exception cref="InvalidOperationException">
public abstract ValueTask<ReadResourceResult?> ReadAsync( /// The <see cref="ReadResourceRequestParams.Uri"/> did not match the <see cref="ProtocolResourceTemplate"/> for this resource,
/// the resource implementation returned <see langword="null"/>, or the resource implementation returned an unsupported result type.
/// </exception>
public abstract ValueTask<ReadResourceResult> ReadAsync(
RequestContext<ReadResourceRequestParams> request, RequestContext<ReadResourceRequestParams> request,
CancellationToken cancellationToken = default); CancellationToken cancellationToken = default);

View file

@ -0,0 +1,42 @@
using Microsoft.Extensions.DependencyInjection;
using ModelContextProtocol.Client;
using ModelContextProtocol.Protocol;
using ModelContextProtocol.Server;
namespace ModelContextProtocol.Tests.Configuration;
public sealed class McpServerResourceRoutingTests(ITestOutputHelper testOutputHelper) : ClientServerTestBase(testOutputHelper)
{
protected override void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder)
{
mcpServerBuilder.WithResources([
McpServerResource.Create(options: new() { UriTemplate = "test://resource/non-templated" } , method: () => "static"),
McpServerResource.Create(options: new() { UriTemplate = "test://resource/{id}" }, method: (string id) => $"template: {id}"),
McpServerResource.Create(options: new() { UriTemplate = "test://params{?a1,a2,a3}" }, method: (string a1, string a2, string a3) => $"params: {a1}, {a2}, {a3}"),
]);
}
[Fact]
public async Task MultipleTemplatedResources_MatchesCorrectResource()
{
// Verify that when multiple templated resources exist, the correct one is matched based on the URI pattern, not just the first one.
// Regression test for https://github.com/modelcontextprotocol/csharp-sdk/issues/821.
await using McpClient client = await CreateMcpClientForServer();
var nonTemplatedResult = await client.ReadResourceAsync("test://resource/non-templated", TestContext.Current.CancellationToken);
Assert.Equal("static", ((TextResourceContents)nonTemplatedResult.Contents[0]).Text);
var templatedResult = await client.ReadResourceAsync("test://resource/12345", TestContext.Current.CancellationToken);
Assert.Equal("template: 12345", ((TextResourceContents)templatedResult.Contents[0]).Text);
var exactTemplatedResult = await client.ReadResourceAsync("test://resource/{id}", TestContext.Current.CancellationToken);
Assert.Equal("template: {id}", ((TextResourceContents)exactTemplatedResult.Contents[0]).Text);
var paramsResult = await client.ReadResourceAsync("test://params?a1=a&a2=b&a3=c", TestContext.Current.CancellationToken);
Assert.Equal("params: a, b, c", ((TextResourceContents)paramsResult.Contents[0]).Text);
var mcpEx = await Assert.ThrowsAsync<McpProtocolException>(async () => await client.ReadResourceAsync("test://params{?a1,a2,a3}", TestContext.Current.CancellationToken));
Assert.Equal(McpErrorCode.InvalidParams, mcpEx.ErrorCode);
Assert.Equal("Request failed (remote): Unknown resource URI: 'test://params{?a1,a2,a3}'", mcpEx.Message);
}
}

View file

@ -142,7 +142,7 @@ public partial class McpServerResourceTests
const string Name = "Hello"; const string Name = "Hello";
McpServerResource t; McpServerResource t;
ReadResourceResult? result; ReadResourceResult result;
McpServer server = new Mock<McpServer>().Object; McpServer server = new Mock<McpServer>().Object;
t = McpServerResource.Create(() => "42", new() { Name = Name }); t = McpServerResource.Create(() => "42", new() { Name = Name });
@ -282,11 +282,12 @@ public partial class McpServerResourceTests
[InlineData("resource://mcp/Hello?arg1=42&arg2=84")] [InlineData("resource://mcp/Hello?arg1=42&arg2=84")]
[InlineData("resource://mcp/Hello?arg1=42&arg2=84&arg3=123")] [InlineData("resource://mcp/Hello?arg1=42&arg2=84&arg3=123")]
[InlineData("resource://mcp/Hello#fragment")] [InlineData("resource://mcp/Hello#fragment")]
public async Task UriTemplate_NonMatchingUri_ReturnsNull(string uri) public async Task UriTemplate_NonMatchingUri_DoesNotMatch(string uri)
{ {
McpServerResource t = McpServerResource.Create((string arg1) => arg1, new() { Name = "Hello" }); McpServerResource t = McpServerResource.Create((string arg1) => arg1, new() { Name = "Hello" });
Assert.Equal("resource://mcp/Hello{?arg1}", t.ProtocolResourceTemplate.UriTemplate); Assert.Equal("resource://mcp/Hello{?arg1}", t.ProtocolResourceTemplate.UriTemplate);
Assert.Null(await t.ReadAsync( Assert.False(t.IsMatch(uri));
await Assert.ThrowsAsync<InvalidOperationException>(async () => await t.ReadAsync(
new RequestContext<ReadResourceRequestParams>(new Mock<McpServer>().Object, CreateTestJsonRpcRequest()) { Params = new() { Uri = uri } }, new RequestContext<ReadResourceRequestParams>(new Mock<McpServer>().Object, CreateTestJsonRpcRequest()) { Params = new() { Uri = uri } },
TestContext.Current.CancellationToken)); TestContext.Current.CancellationToken));
} }
@ -337,7 +338,7 @@ public partial class McpServerResourceTests
McpServerResource t = McpServerResource.Create((string? arg1 = null, int? arg2 = null) => arg1 + arg2, new() { Name = "Hello" }); McpServerResource t = McpServerResource.Create((string? arg1 = null, int? arg2 = null) => arg1 + arg2, new() { Name = "Hello" });
Assert.Equal("resource://mcp/Hello{?arg1,arg2}", t.ProtocolResourceTemplate.UriTemplate); Assert.Equal("resource://mcp/Hello{?arg1,arg2}", t.ProtocolResourceTemplate.UriTemplate);
ReadResourceResult? result; ReadResourceResult result;
result = await t.ReadAsync( result = await t.ReadAsync(
new RequestContext<ReadResourceRequestParams>(new Mock<McpServer>().Object, CreateTestJsonRpcRequest()) { Params = new() { Uri = "resource://mcp/Hello" } }, new RequestContext<ReadResourceRequestParams>(new Mock<McpServer>().Object, CreateTestJsonRpcRequest()) { Params = new() { Uri = "resource://mcp/Hello" } },

View file

@ -626,7 +626,7 @@ public class McpServerTests : LoggedTest
TestContext.Current.CancellationToken TestContext.Current.CancellationToken
); );
var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); var error = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken);
Assert.NotNull(error); Assert.NotNull(error);
Assert.NotNull(error.Error); Assert.NotNull(error.Error);
Assert.Equal((int)errorCode, error.Error.Code); Assert.Equal((int)errorCode, error.Error.Code);
@ -662,7 +662,7 @@ public class McpServerTests : LoggedTest
} }
); );
var response = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(5)); var response = await receivedMessage.Task.WaitAsync(TimeSpan.FromSeconds(10));
Assert.NotNull(response); Assert.NotNull(response);
assertResult(server, response.Result); assertResult(server, response.Result);