Skip to content

Commit 81664c3

Browse files
authored
Fix reserved expansion (e.g. "{+var}") when matching resources (#1142)
1 parent 6ca7cde commit 81664c3

File tree

3 files changed

+967
-50
lines changed

3 files changed

+967
-50
lines changed

src/ModelContextProtocol.Core/UriTemplate.cs

Lines changed: 65 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ internal static partial class UriTemplate
6767
public static Regex CreateParser(string uriTemplate)
6868
{
6969
DefaultInterpolatedStringHandler pattern = new(0, 0, CultureInfo.InvariantCulture, stackalloc char[256]);
70-
pattern.AppendFormatted('^');
70+
pattern.AppendLiteral("^");
7171

7272
int lastIndex = 0;
7373
for (Match m = UriTemplateExpression().Match(uriTemplate); m.Success; m = m.NextMatch())
@@ -84,17 +84,20 @@ public static Regex CreateParser(string uriTemplate)
8484

8585
switch (m.Groups["operator"].Value)
8686
{
87-
case "#": AppendExpression(ref pattern, paramNames, '#', "[^,]+"); break;
88-
case "/": AppendExpression(ref pattern, paramNames, '/', "[^/?]+"); break;
89-
default: AppendExpression(ref pattern, paramNames, null, "[^/?&]+"); break;
90-
87+
case "+": AppendExpression(ref pattern, paramNames, null, "[^?&#]*"); break;
88+
case "#": AppendExpression(ref pattern, paramNames, '#', ".*"); break;
89+
case ".": AppendExpression(ref pattern, paramNames, '.', "[^/?#]*"); break;
90+
case "/": AppendExpression(ref pattern, paramNames, '/', "[^/?#]*"); break;
91+
default: AppendExpression(ref pattern, paramNames, null, "[^/?&#]*"); break;
92+
9193
case "?": AppendQueryExpression(ref pattern, paramNames, '?'); break;
9294
case "&": AppendQueryExpression(ref pattern, paramNames, '&'); break;
95+
case ";": AppendPathParameterExpression(ref pattern, paramNames); break;
9396
}
9497
}
9598

9699
pattern.AppendFormatted(Regex.Escape(uriTemplate.Substring(lastIndex)));
97-
pattern.AppendFormatted('$');
100+
pattern.AppendLiteral("$");
98101

99102
return new Regex(
100103
pattern.ToStringAndClear(),
@@ -113,63 +116,101 @@ static void AppendQueryExpression(ref DefaultInterpolatedStringHandler pattern,
113116
{
114117
Debug.Assert(prefix is '?' or '&');
115118

116-
pattern.AppendFormatted("(?:\\");
119+
pattern.AppendLiteral("(?:\\");
117120
pattern.AppendFormatted(prefix);
118121

119122
if (paramNames.Count > 0)
120123
{
121124
AppendParameter(ref pattern, paramNames[0]);
122125
for (int i = 1; i < paramNames.Count; i++)
123126
{
124-
pattern.AppendFormatted("\\&?");
127+
pattern.AppendLiteral("\\&?");
125128
AppendParameter(ref pattern, paramNames[i]);
126129
}
127130

128131
static void AppendParameter(ref DefaultInterpolatedStringHandler pattern, string paramName)
129132
{
130133
paramName = Regex.Escape(paramName);
131-
pattern.AppendFormatted("(?:");
134+
pattern.AppendLiteral("(?:");
132135
pattern.AppendFormatted(paramName);
133-
pattern.AppendFormatted("=(?<");
136+
pattern.AppendLiteral("=(?<");
134137
pattern.AppendFormatted(paramName);
135-
pattern.AppendFormatted(">[^/?&]+))?");
138+
pattern.AppendLiteral(">[^/?&]*))?");
136139
}
137140
}
138141

139-
pattern.AppendFormatted(")?");
142+
pattern.AppendLiteral(")?");
140143
}
141144

142145
// Chooses a regex character‐class (`valueChars`) based on the initial `prefix` to define which
143146
// characters make up a parameter value. Then, for each name in `paramNames`, it optionally
144147
// appends the escaped `prefix` (only on the first parameter, then switches to ','), and
145148
// adds an optional named capture group `(?<paramName>valueChars)` to match and capture that value.
149+
// Note: For "+" (reserved expansion) operator, prefix is null but valueChars allows "/" characters.
150+
// Note: For "." (label expansion) operator, the separator is "." instead of ",".
146151
static void AppendExpression(ref DefaultInterpolatedStringHandler pattern, List<string> paramNames, char? prefix, string valueChars)
147152
{
148-
Debug.Assert(prefix is '#' or '/' or null);
153+
Debug.Assert(prefix is '#' or '/' or '.' or null);
149154

150155
if (paramNames.Count > 0)
151156
{
152157
if (prefix is not null)
153158
{
154-
pattern.AppendFormatted('\\');
159+
pattern.AppendLiteral("\\");
155160
pattern.AppendFormatted(prefix);
156-
pattern.AppendFormatted('?');
161+
pattern.AppendLiteral("?");
157162
}
158163

159164
AppendParameter(ref pattern, paramNames[0], valueChars);
165+
166+
// For label expansion (.), the separator between values is also a dot
167+
// For path segment expansion (/), the separator between values is also a slash
168+
string separator = prefix switch
169+
{
170+
'.' => "\\.",
171+
'/' => "\\/",
172+
_ => "\\,"
173+
};
160174
for (int i = 1; i < paramNames.Count; i++)
161175
{
162-
pattern.AppendFormatted("\\,?");
176+
pattern.AppendFormatted(separator);
177+
pattern.AppendLiteral("?");
163178
AppendParameter(ref pattern, paramNames[i], valueChars);
164179
}
165180

166181
static void AppendParameter(ref DefaultInterpolatedStringHandler pattern, string paramName, string valueChars)
167182
{
168-
pattern.AppendFormatted("(?<");
183+
pattern.AppendLiteral("(?<");
169184
pattern.AppendFormatted(Regex.Escape(paramName));
170-
pattern.AppendFormatted('>');
185+
pattern.AppendLiteral(">");
171186
pattern.AppendFormatted(valueChars);
172-
pattern.AppendFormatted(")?");
187+
pattern.AppendLiteral(")?");
188+
}
189+
}
190+
}
191+
192+
// Appends a regex fragment for path-style parameter expansion (;).
193+
// Format: ;name=value or ;name (if value is empty), separated by semicolons.
194+
// Each parameter is made optional and captured by a named group.
195+
static void AppendPathParameterExpression(ref DefaultInterpolatedStringHandler pattern, List<string> paramNames)
196+
{
197+
if (paramNames.Count > 0)
198+
{
199+
AppendParameter(ref pattern, paramNames[0]);
200+
for (int i = 1; i < paramNames.Count; i++)
201+
{
202+
AppendParameter(ref pattern, paramNames[i]);
203+
}
204+
205+
static void AppendParameter(ref DefaultInterpolatedStringHandler pattern, string paramName)
206+
{
207+
// Match ;name or ;name=value
208+
paramName = Regex.Escape(paramName);
209+
pattern.AppendLiteral("(?:;");
210+
pattern.AppendFormatted(paramName);
211+
pattern.AppendLiteral("(?:=(?<");
212+
pattern.AppendFormatted(paramName);
213+
pattern.AppendLiteral(">[^;/?&]*))?)?");
173214
}
174215
}
175216
}
@@ -363,7 +404,7 @@ value as string ??
363404
}
364405
}
365406

366-
if (expansions.Count > 0 &&
407+
if (expansions.Count > 0 &&
367408
(modifierBehavior.PrefixEmptyExpansions || !expansions.All(string.IsNullOrEmpty)))
368409
{
369410
builder.AppendLiteral(modifierBehavior.Prefix);
@@ -435,7 +476,7 @@ static void AppendHex(ref DefaultInterpolatedStringHandler builder, char c)
435476

436477
if (c <= 0x7F)
437478
{
438-
builder.AppendFormatted('%');
479+
builder.AppendLiteral("%");
439480
builder.AppendFormatted(hexDigits[c >> 4]);
440481
builder.AppendFormatted(hexDigits[c & 0xF]);
441482
}
@@ -448,7 +489,7 @@ static void AppendHex(ref DefaultInterpolatedStringHandler builder, char c)
448489
foreach (byte b in Encoding.UTF8.GetBytes([c]))
449490
#endif
450491
{
451-
builder.AppendFormatted('%');
492+
builder.AppendLiteral("%");
452493
builder.AppendFormatted(hexDigits[b >> 4]);
453494
builder.AppendFormatted(hexDigits[b & 0xF]);
454495
}
@@ -460,13 +501,13 @@ static void AppendHex(ref DefaultInterpolatedStringHandler builder, char c)
460501
/// Defines an equality comparer for Uri templates as follows:
461502
/// 1. Non-templated Uris use regular System.Uri equality comparison (host name is case insensitive).
462503
/// 2. Templated Uris use regular string equality.
463-
///
504+
///
464505
/// We do this because non-templated resources are looked up directly from the resource dictionary
465506
/// and we need to make sure equality is implemented correctly. Templated Uris are resolved in a
466507
/// fallback step using linear traversal of the resource dictionary, so their equality is only
467508
/// there to distinguish between different templates.
468509
/// </summary>
469-
public sealed class UriTemplateComparer : IEqualityComparer<string>
510+
internal sealed class UriTemplateComparer : IEqualityComparer<string>
470511
{
471512
public static IEqualityComparer<string> Instance { get; } = new UriTemplateComparer();
472513

tests/ModelContextProtocol.Tests/ClientServerTestBase.cs

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,36 +13,55 @@ public abstract class ClientServerTestBase : LoggedTest, IAsyncDisposable
1313
{
1414
private readonly Pipe _clientToServerPipe = new();
1515
private readonly Pipe _serverToClientPipe = new();
16-
private readonly IMcpServerBuilder _builder;
17-
private readonly CancellationTokenSource _cts;
18-
private readonly Task _serverTask;
16+
private readonly CancellationTokenSource _cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
17+
private Task _serverTask = Task.CompletedTask;
1918

20-
public ClientServerTestBase(ITestOutputHelper testOutputHelper)
19+
public ClientServerTestBase(ITestOutputHelper testOutputHelper, bool startServer = true)
2120
: base(testOutputHelper)
2221
{
23-
ServiceCollection sc = new();
24-
sc.AddLogging();
25-
sc.AddSingleton(XunitLoggerProvider);
26-
sc.AddSingleton<ILoggerProvider>(MockLoggerProvider);
27-
_builder = sc
22+
ServiceCollection.AddLogging();
23+
ServiceCollection.AddSingleton(XunitLoggerProvider);
24+
ServiceCollection.AddSingleton<ILoggerProvider>(MockLoggerProvider);
25+
McpServerBuilder = ServiceCollection
2826
.AddMcpServer()
2927
.WithStreamServerTransport(_clientToServerPipe.Reader.AsStream(), _serverToClientPipe.Writer.AsStream());
30-
ConfigureServices(sc, _builder);
31-
ServiceProvider = sc.BuildServiceProvider(validateScopes: true);
3228

33-
_cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
34-
Server = ServiceProvider.GetRequiredService<McpServer>();
35-
_serverTask = Server.RunAsync(_cts.Token);
29+
ConfigureServices(ServiceCollection, McpServerBuilder);
30+
31+
if (startServer)
32+
{
33+
StartServer();
34+
}
3635
}
3736

38-
protected McpServer Server { get; }
37+
protected ServiceCollection ServiceCollection { get; } = [];
38+
39+
protected IMcpServerBuilder McpServerBuilder { get; }
40+
41+
protected McpServer Server
42+
{
43+
get => field ?? throw new InvalidOperationException("You must call StartServer first.");
44+
private set => field = value;
45+
}
3946

40-
protected IServiceProvider ServiceProvider { get; }
47+
protected ServiceProvider ServiceProvider
48+
{
49+
get => field ?? throw new InvalidOperationException("You must call StartServer first.");
50+
private set => field = value;
51+
}
4152

4253
protected virtual void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder)
4354
{
4455
}
4556

57+
protected McpServer StartServer()
58+
{
59+
ServiceProvider = ServiceCollection.BuildServiceProvider(validateScopes: true);
60+
Server = ServiceProvider.GetRequiredService<McpServer>();
61+
_serverTask = Server.RunAsync(_cts.Token);
62+
return Server;
63+
}
64+
4665
public async ValueTask DisposeAsync()
4766
{
4867
await _cts.CancelAsync();

0 commit comments

Comments
 (0)