Skip to content

Commit ef7281a

Browse files
tarekghtarekgh
committed
Fix SEP-2243 spec compliance gaps and add comprehensive tests
Address multiple specification compliance issues identified during review: Source fixes: - Fix tab (0x09) encoding: tabs now trigger Base64 encoding per spec - Add client-side tool schema validation: tools with invalid x-mcp-header annotations (non-object schemas, missing properties) are filtered out - Remove server empty-header early-return that skipped validation - Fix numeric precision loss: use GetRawText()/ToJsonString() instead of GetDouble()/GetValue<double>() for header value extraction - Implement proper version gating: only DRAFT-2026-v1 requires header validation on server side (client always sends headers unconditionally) - Add server-side invalid character validation for header values Test additions: - 22 new integration tests in Sep2243HeaderTests.cs covering encoding, validation, tool filtering, and end-to-end header scenarios - 4 new server conformance tests for draft version header validation - 2 new client conformance tests for tool filtering and header sending - 2 new unit tests for tab encoding behavior - 1 new theory for SupportsStandardHeaders version gating - Fix ConformanceClient to handle toolCalls array format and add prompts/list + prompts/get calls for http-standard-headers scenario Other: - Add *.lscache to .gitignore
1 parent 4164474 commit ef7281a

13 files changed

Lines changed: 883 additions & 26 deletions

File tree

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
# Cake tools
22
/[Tt]ools/
33

4+
# Language server cache
5+
*.lscache
6+
47
# Build output
58
[Bb]uildArtifacts/
69
# Build results

‎src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,7 @@ internal static bool ValidateMcpHeaders(HttpContext context, JsonRpcMessage mess
561561
{
562562
// Only validate for protocol versions that support standard headers.
563563
var protocolVersion = context.Request.Headers[McpProtocolVersionHeaderName].ToString();
564-
if (string.IsNullOrEmpty(protocolVersion) ||
565-
string.Compare(protocolVersion, McpHttpHeaders.MinVersionForStandardHeaders, StringComparison.Ordinal) < 0)
564+
if (!McpHttpHeaders.SupportsStandardHeaders(protocolVersion))
566565
{
567566
errorMessage = null;
568567
return true;
@@ -724,9 +723,13 @@ argForMissing is not null &&
724723
}
725724

726725
var actualHeaderValue = context.Request.Headers[fullHeaderName].ToString();
727-
if (string.IsNullOrEmpty(actualHeaderValue))
726+
727+
// Validate the raw header value for invalid characters per SEP.
728+
// Servers MUST reject headers containing characters outside the valid HTTP header value range.
729+
if (!IsValidHeaderValue(actualHeaderValue))
728730
{
729-
continue;
731+
errorMessage = $"Header mismatch: {fullHeaderName} header contains invalid characters.";
732+
return false;
730733
}
731734

732735
var decodedActual = Client.McpHeaderEncoder.DecodeValue(actualHeaderValue);
@@ -768,6 +771,26 @@ argForMissing is not null &&
768771
return null;
769772
}
770773

774+
/// <summary>
775+
/// Validates that a header value contains only characters allowed in HTTP header field values
776+
/// per RFC 9110: visible ASCII (0x21-0x7E), space (0x20), and horizontal tab (0x09).
777+
/// </summary>
778+
private static bool IsValidHeaderValue(string value)
779+
{
780+
foreach (char c in value)
781+
{
782+
if (c < 0x20 || c > 0x7E)
783+
{
784+
if (c != '\t')
785+
{
786+
return false;
787+
}
788+
}
789+
}
790+
791+
return true;
792+
}
793+
771794
private static string? ConvertJsonNodeToHeaderValue(System.Text.Json.Nodes.JsonNode node)
772795
{
773796
if (node is not System.Text.Json.Nodes.JsonValue jsonValue)
@@ -778,7 +801,7 @@ argForMissing is not null &&
778801
object? value = jsonValue.GetValueKind() switch
779802
{
780803
System.Text.Json.JsonValueKind.String => jsonValue.GetValue<string>(),
781-
System.Text.Json.JsonValueKind.Number => jsonValue.GetValue<double>(),
804+
System.Text.Json.JsonValueKind.Number => jsonValue.ToJsonString(),
782805
System.Text.Json.JsonValueKind.True => true,
783806
System.Text.Json.JsonValueKind.False => false,
784807
_ => null

‎src/ModelContextProtocol.Core/Client/McpClient.Methods.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,14 @@ public async ValueTask<IList<McpClientTool>> ListToolsAsync(
184184
tools ??= new(toolResults.Tools.Count);
185185
foreach (var tool in toolResults.Tools)
186186
{
187+
// Validate x-mcp-header annotations per SEP-2243.
188+
// Clients MUST exclude tools with invalid annotations and SHOULD log a warning.
189+
if (!McpHeaderExtractor.ValidateToolSchema(tool, out var rejectionReason))
190+
{
191+
OnToolRejected(tool, rejectionReason!);
192+
continue;
193+
}
194+
187195
OnToolDiscovered(tool);
188196
tools.Add(new(this, tool, options?.JsonSerializerOptions));
189197
}
@@ -206,6 +214,15 @@ internal virtual void OnToolDiscovered(Tool tool)
206214
{
207215
}
208216

217+
/// <summary>
218+
/// Called when a tool definition is rejected due to invalid <c>x-mcp-header</c> annotations.
219+
/// </summary>
220+
/// <param name="tool">The tool that was rejected.</param>
221+
/// <param name="reason">The reason the tool was rejected.</param>
222+
internal virtual void OnToolRejected(Tool tool, string reason)
223+
{
224+
}
225+
209226
/// <summary>
210227
/// Retrieves a list of available tools from the server.
211228
/// </summary>

‎src/ModelContextProtocol.Core/Client/McpClientImpl.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,11 @@ internal override void OnToolDiscovered(Tool tool)
661661
_toolCache[tool.Name] = tool;
662662
}
663663

664+
internal override void OnToolRejected(Tool tool, string reason)
665+
{
666+
LogToolRejected(tool.Name, reason);
667+
}
668+
664669
/// <inheritdoc/>
665670
public override IAsyncDisposable RegisterNotificationHandler(string method, Func<JsonRpcNotification, CancellationToken, ValueTask> handler)
666671
=> _sessionHandler.RegisterNotificationHandler(method, handler);
@@ -708,4 +713,7 @@ public override async ValueTask DisposeAsync()
708713
[LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} client resumed existing session.")]
709714
private partial void LogClientSessionResumed(string endpointName);
710715

716+
[LoggerMessage(Level = LogLevel.Warning, Message = "Tool '{ToolName}' excluded from tools/list: {Reason}")]
717+
private partial void LogToolRejected(string toolName, string reason);
718+
711719
}

‎src/ModelContextProtocol.Core/Client/McpHeaderEncoder.cs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace ModelContextProtocol.Client;
1515
/// <para>
1616
/// Encoding rules:
1717
/// <list type="bullet">
18-
/// <item><description>Plain ASCII values (0x20-0x7E, tab 0x09): sent as-is</description></item>
18+
/// <item><description>Plain ASCII values (0x20-0x7E): sent as-is</description></item>
1919
/// <item><description>Values with leading/trailing whitespace: Base64 encoded with <c>=?base64?{value}?=</c> wrapper</description></item>
2020
/// <item><description>Non-ASCII characters: Base64 encoded</description></item>
2121
/// <item><description>Control characters: Base64 encoded</description></item>
@@ -129,13 +129,11 @@ private static bool RequiresBase64Encoding(string value)
129129

130130
foreach (char c in value)
131131
{
132-
// Valid HTTP header field value characters: visible ASCII (0x21-0x7E), space (0x20), tab (0x09)
132+
// Valid HTTP header field value characters per SEP: visible ASCII (0x21-0x7E) and space (0x20).
133+
// All control characters (0x00-0x1F, 0x7F), including tab, must be Base64-encoded.
133134
if (c < 0x20 || c > 0x7E)
134135
{
135-
if (c != '\t')
136-
{
137-
return true;
138-
}
136+
return true;
139137
}
140138
}
141139

‎src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Net.Http.Headers;
22
using System.Text.Json;
3+
using Microsoft.Extensions.Logging;
34
using ModelContextProtocol.Protocol;
45

56
namespace ModelContextProtocol.Client;
@@ -37,7 +38,8 @@ public static void AddParameterHeaders(
3738

3839
foreach (var property in properties.EnumerateObject())
3940
{
40-
if (!property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement))
41+
if (property.Value.ValueKind != JsonValueKind.Object ||
42+
!property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement))
4143
{
4244
continue;
4345
}
@@ -73,12 +75,87 @@ public static void AddParameterHeaders(
7375
object? value = element.ValueKind switch
7476
{
7577
JsonValueKind.String => element.GetString(),
76-
JsonValueKind.Number => element.GetDouble(),
78+
JsonValueKind.Number => element.GetRawText(),
7779
JsonValueKind.True => true,
7880
JsonValueKind.False => false,
7981
_ => null
8082
};
8183

8284
return McpHeaderEncoder.EncodeValue(value);
8385
}
86+
87+
/// <summary>
88+
/// Validates a tool's <c>inputSchema</c> for valid <c>x-mcp-header</c> annotations.
89+
/// Returns <see langword="true"/> if the tool is valid; <see langword="false"/> with a reason if it should be rejected.
90+
/// </summary>
91+
internal static bool ValidateToolSchema(Tool tool, out string? rejectionReason)
92+
{
93+
rejectionReason = null;
94+
95+
if (tool.InputSchema.ValueKind != JsonValueKind.Object ||
96+
!tool.InputSchema.TryGetProperty("properties", out var properties) ||
97+
properties.ValueKind != JsonValueKind.Object)
98+
{
99+
return true;
100+
}
101+
102+
var headerNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
103+
104+
foreach (var property in properties.EnumerateObject())
105+
{
106+
// Skip properties whose schema is not an object (e.g., boolean `true`/`false` schemas)
107+
if (property.Value.ValueKind != JsonValueKind.Object ||
108+
!property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement))
109+
{
110+
continue;
111+
}
112+
113+
// x-mcp-header value must be a string
114+
if (headerNameElement.ValueKind != JsonValueKind.String)
115+
{
116+
rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' is not a string.";
117+
return false;
118+
}
119+
120+
var headerName = headerNameElement.GetString();
121+
122+
// MUST NOT be empty
123+
if (string.IsNullOrEmpty(headerName))
124+
{
125+
rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' is empty.";
126+
return false;
127+
}
128+
129+
// MUST contain only ASCII characters (0x21-0x7E) excluding space and colon
130+
foreach (char c in headerName!)
131+
{
132+
if (c < 0x21 || c > 0x7E || c == ':')
133+
{
134+
rejectionReason = $"Tool '{tool.Name}': x-mcp-header '{headerName}' contains invalid character '{c}' (0x{(int)c:X2}).";
135+
return false;
136+
}
137+
}
138+
139+
// MUST be case-insensitively unique
140+
if (!headerNames.Add(headerName))
141+
{
142+
rejectionReason = $"Tool '{tool.Name}': duplicate x-mcp-header name '{headerName}' (case-insensitive).";
143+
return false;
144+
}
145+
146+
// MUST only be applied to primitive types (string, number, boolean)
147+
if (property.Value.TryGetProperty("type", out var typeElement) &&
148+
typeElement.ValueKind == JsonValueKind.String)
149+
{
150+
var typeName = typeElement.GetString();
151+
if (typeName is not ("string" or "number" or "integer" or "boolean"))
152+
{
153+
rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' has non-primitive type '{typeName}'.";
154+
return false;
155+
}
156+
}
157+
}
158+
159+
return true;
160+
}
84161
}

‎src/ModelContextProtocol.Core/Protocol/McpHttpHeaders.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,18 @@ public static class McpHttpHeaders
6161
/// definition for the current request, enabling the transport to add custom parameter headers.
6262
/// </summary>
6363
internal const string ToolContextKey = "Mcp.Tool";
64+
65+
/// <summary>
66+
/// Protocol versions that require standard MCP request headers (Mcp-Method, Mcp-Name).
67+
/// </summary>
68+
private static readonly HashSet<string> s_versionsWithStandardHeaders = new(StringComparer.Ordinal)
69+
{
70+
MinVersionForStandardHeaders,
71+
};
72+
73+
/// <summary>
74+
/// Returns <see langword="true"/> if the given protocol version requires standard MCP request headers.
75+
/// </summary>
76+
public static bool SupportsStandardHeaders(string? protocolVersion)
77+
=> !string.IsNullOrEmpty(protocolVersion) && s_versionsWithStandardHeaders.Contains(protocolVersion!);
6478
}

0 commit comments

Comments
 (0)