From ab647d89a2393e574f125a8d4481bb526333d911 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Mon, 1 Jun 2026 16:28:19 -0700 Subject: [PATCH 1/2] Align x-mcp-header implementation with SEP-2243 spec clarifications - Enforce RFC 9110 tchar validation for header names using SearchValues on .NET 8+ and bitmap lookup on netstandard2.0 - Remove 'number' from allowed x-mcp-header types (only string, integer, boolean are permitted); reject float/double/decimal in [McpHeader] - Add base64 sentinel collision check in McpHeaderEncoder.RequiresBase64Encoding so literal values resembling =?base64?...?= are encoded to avoid confusion - Fix DecodeValue to use case-sensitive prefix matching per spec requirement that sentinel markers must appear exactly as shown (lowercase) - Support nested property traversal for x-mcp-header validation and extraction on both client and server sides - Share tchar validation logic between McpHeaderAttribute and McpHeaderExtractor via internal FindFirstNonTchar method - Add recursive ValidateCustomParamHeadersFromProperties in StreamableHttpHandler for server-side nested property header/body comparison - Add 28 new tests covering tchar validation, sentinel collision encoding, number type rejection, nested property handling, and case-sensitive decoding --- .../StreamableHttpHandler.cs | 33 ++++ .../Client/McpClient.Methods.cs | 3 +- .../Client/McpHeaderExtractor.cs | 169 ++++++++++++++++-- .../Protocol/McpHeaderEncoder.cs | 14 +- .../Server/AIFunctionMcpServerTool.cs | 9 +- .../Server/McpHeaderAttribute.cs | 27 ++- .../Client/McpHeaderEncoderTests.cs | 36 +++- .../McpHeaderExtractorValidationTests.cs | 152 ++++++++++++++++ .../Server/McpHeaderAttributeTests.cs | 20 +++ 9 files changed, 422 insertions(+), 41 deletions(-) create mode 100644 tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs diff --git a/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs b/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs index 9763a6cf5..0719e5794 100644 --- a/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs +++ b/src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs @@ -703,8 +703,41 @@ private static bool ValidateCustomParamHeaders( // Check that every x-mcp-header annotated parameter has a corresponding header, // that the header value is validly encoded, and that it matches the body value. + return ValidateCustomParamHeadersFromProperties(context, properties, arguments, out errorMessage); + } + + /// + /// Recursively validates x-mcp-header annotated properties at any nesting depth. + /// + private static bool ValidateCustomParamHeadersFromProperties( + HttpContext context, + System.Text.Json.JsonElement properties, + System.Text.Json.Nodes.JsonNode? arguments, + [NotNullWhen(false)] out string? errorMessage) + { foreach (var property in properties.EnumerateObject()) { + if (property.Value.ValueKind != System.Text.Json.JsonValueKind.Object) + { + continue; + } + + // Recurse into nested object properties + if (property.Value.TryGetProperty("properties", out var nestedProperties) && + nestedProperties.ValueKind == System.Text.Json.JsonValueKind.Object) + { + System.Text.Json.Nodes.JsonNode? nestedArgs = null; + if (arguments is System.Text.Json.Nodes.JsonObject parentObj) + { + parentObj.TryGetPropertyValue(property.Name, out nestedArgs); + } + + if (!ValidateCustomParamHeadersFromProperties(context, nestedProperties, nestedArgs, out errorMessage)) + { + return false; + } + } + if (!property.Value.TryGetProperty("x-mcp-header", out var headerNameElement)) { continue; diff --git a/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs b/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs index f04c32ffd..59ee6edc2 100644 --- a/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs +++ b/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs @@ -187,7 +187,8 @@ public async ValueTask> ListToolsAsync( foreach (var tool in toolResults.Tools) { // Validate x-mcp-header annotations per SEP-2243. - // Clients MUST exclude tools with invalid annotations and SHOULD log a warning. + // Streamable HTTP clients MUST exclude tools with invalid annotations; + // non-HTTP clients MAY also reject them for safety. if (!McpHeaderExtractor.ValidateToolSchema(tool, out var rejectionReason)) { ToolRejected?.Invoke(tool, rejectionReason!); diff --git a/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs b/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs index 349168f04..2206a98cc 100644 --- a/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs +++ b/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs @@ -1,5 +1,8 @@ using System.Net.Http.Headers; using System.Text.Json; +#if NET +using System.Buffers; +#endif using Microsoft.Extensions.Logging; using ModelContextProtocol.Protocol; @@ -36,10 +39,35 @@ public static void AddParameterHeaders( return; } + AddParameterHeadersFromProperties(headers, properties, arguments.Value); + } + + /// + /// Recursively extracts parameter values from properties at any nesting depth + /// and adds them as HTTP headers. + /// + private static void AddParameterHeadersFromProperties( + HttpRequestHeaders headers, + JsonElement properties, + JsonElement arguments) + { foreach (var property in properties.EnumerateObject()) { - if (property.Value.ValueKind != JsonValueKind.Object || - !property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement)) + if (property.Value.ValueKind != JsonValueKind.Object) + { + continue; + } + + // Recurse into nested object properties + if (property.Value.TryGetProperty("properties", out var nestedProperties) && + nestedProperties.ValueKind == JsonValueKind.Object && + arguments.TryGetProperty(property.Name, out var nestedArgs) && + nestedArgs.ValueKind == JsonValueKind.Object) + { + AddParameterHeadersFromProperties(headers, nestedProperties, nestedArgs); + } + + if (!property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement)) { continue; } @@ -51,7 +79,7 @@ public static void AddParameterHeaders( } // Look for the corresponding argument value - if (!arguments.Value.TryGetProperty(property.Name, out var argValue)) + if (!arguments.TryGetProperty(property.Name, out var argValue)) { continue; } @@ -86,12 +114,35 @@ internal static bool ValidateToolSchema(Tool tool, out string? rejectionReason) } var headerNames = new HashSet(StringComparer.OrdinalIgnoreCase); + return ValidateProperties(tool, properties, headerNames, out rejectionReason); + } + + /// + /// Recursively validates properties at any nesting depth for valid x-mcp-header annotations. + /// + private static bool ValidateProperties(Tool tool, JsonElement properties, HashSet headerNames, out string? rejectionReason) + { + rejectionReason = null; foreach (var property in properties.EnumerateObject()) { // Skip properties whose schema is not an object (e.g., boolean `true`/`false` schemas) - if (property.Value.ValueKind != JsonValueKind.Object || - !property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement)) + if (property.Value.ValueKind != JsonValueKind.Object) + { + continue; + } + + // Recurse into nested object properties + if (property.Value.TryGetProperty("properties", out var nestedProperties) && + nestedProperties.ValueKind == JsonValueKind.Object) + { + if (!ValidateProperties(tool, nestedProperties, headerNames, out rejectionReason)) + { + return false; + } + } + + if (!property.Value.TryGetProperty(XMcpHeaderProperty, out var headerNameElement)) { continue; } @@ -112,29 +163,30 @@ internal static bool ValidateToolSchema(Tool tool, out string? rejectionReason) return false; } - // MUST contain only ASCII characters (0x21-0x7E) excluding space and colon - foreach (char c in headerName!) + // MUST match HTTP field-name token syntax (1*tchar, RFC 9110 Section 5.1) + // MUST NOT contain control characters including CR and LF + int invalidIdx = FindFirstNonTchar(headerName!); + if (invalidIdx >= 0) { - if (c < 0x21 || c > 0x7E || c == ':') - { - rejectionReason = $"Tool '{tool.Name}': x-mcp-header '{headerName}' contains invalid character '{c}' (0x{(int)c:X2})."; - return false; - } + char c = headerName![invalidIdx]; + rejectionReason = $"Tool '{tool.Name}': x-mcp-header '{headerName}' contains invalid character '{c}' (0x{(int)c:X2})."; + return false; } // MUST be case-insensitively unique - if (!headerNames.Add(headerName)) + if (!headerNames.Add(headerName!)) { rejectionReason = $"Tool '{tool.Name}': duplicate x-mcp-header name '{headerName}' (case-insensitive)."; return false; } - // MUST only be applied to primitive types (string, number, boolean) + // MUST only be applied to primitive types (integer, string, boolean). + // Parameters with type "number" are not permitted. if (property.Value.TryGetProperty("type", out var typeElement) && typeElement.ValueKind == JsonValueKind.String) { var typeName = typeElement.GetString(); - if (typeName is not ("string" or "number" or "integer" or "boolean")) + if (typeName is not ("string" or "integer" or "boolean")) { rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' has non-primitive type '{typeName}'."; return false; @@ -144,4 +196,91 @@ internal static bool ValidateToolSchema(Tool tool, out string? rejectionReason) return true; } + + // Valid HTTP token characters (tchar) per RFC 9110 Section 5.6.2: + // tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / + // "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA + private const string TcharChars = "!#$%&'*+-.^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + +#if NET + private static readonly SearchValues s_tcharValues = SearchValues.Create(TcharChars); + + /// + /// Returns if every character in is a valid + /// HTTP token character (tchar) per RFC 9110 Section 5.6.2. + /// + private static bool IsValidTcharString(string value) => + value.AsSpan().IndexOfAnyExcept(s_tcharValues) < 0; + + internal static int FindFirstNonTchar(string value) => + value.AsSpan().IndexOfAnyExcept(s_tcharValues); +#else + // Bitmap for O(1) tchar lookup. All valid chars are in 0x21-0x7E range, + // so two ulongs (128 bits) cover the entire ASCII range. + // _tcharBitmapLo covers chars 0-63, _tcharBitmapHi covers chars 64-127. + private static readonly ulong s_tcharBitmapLo = ComputeBitmapLo(); + private static readonly ulong s_tcharBitmapHi = ComputeBitmapHi(); + + private static ulong ComputeBitmapLo() + { + ulong bitmap = 0; + foreach (char c in TcharChars) + { + if (c < 64) + { + bitmap |= 1UL << c; + } + } + return bitmap; + } + + private static ulong ComputeBitmapHi() + { + ulong bitmap = 0; + foreach (char c in TcharChars) + { + if (c >= 64) + { + bitmap |= 1UL << (c - 64); + } + } + return bitmap; + } + + private static bool IsTchar(char c) + { + if (c >= 128) + { + return false; + } + + return c < 64 + ? (s_tcharBitmapLo & (1UL << c)) != 0 + : (s_tcharBitmapHi & (1UL << (c - 64))) != 0; + } + + private static bool IsValidTcharString(string value) + { + foreach (char c in value) + { + if (!IsTchar(c)) + { + return false; + } + } + return true; + } + + internal static int FindFirstNonTchar(string value) + { + for (int i = 0; i < value.Length; i++) + { + if (!IsTchar(value[i])) + { + return i; + } + } + return -1; + } +#endif } diff --git a/src/ModelContextProtocol.Core/Protocol/McpHeaderEncoder.cs b/src/ModelContextProtocol.Core/Protocol/McpHeaderEncoder.cs index c31e93388..845f14c5c 100644 --- a/src/ModelContextProtocol.Core/Protocol/McpHeaderEncoder.cs +++ b/src/ModelContextProtocol.Core/Protocol/McpHeaderEncoder.cs @@ -121,9 +121,9 @@ public static class McpHeaderEncoder return headerValue; } - // Check for Base64 wrapper. The spec defines the prefix as lowercase "=?base64?" - // but we match case-insensitively for robustness against non-conforming senders. - if (headerValue.StartsWith(Base64Prefix, StringComparison.OrdinalIgnoreCase) && + // Check for Base64 wrapper. The spec requires the sentinel markers to be + // case-sensitive and exactly lowercase per SEP-2243. + if (headerValue.StartsWith(Base64Prefix, StringComparison.Ordinal) && headerValue.EndsWith(Base64Suffix, StringComparison.Ordinal)) { var base64Content = headerValue.Substring( @@ -221,6 +221,14 @@ private static bool RequiresBase64Encoding(string value) return true; } + // Avoid sentinel collision: if the value matches the base64 wrapper pattern, + // it must be encoded to prevent ambiguity during decoding. + if (value.StartsWith(Base64Prefix, StringComparison.Ordinal) && + value.EndsWith(Base64Suffix, StringComparison.Ordinal)) + { + return true; + } + foreach (char c in value) { // Valid HTTP header field value characters per SEP: visible ASCII (0x21-0x7E) and space (0x20). diff --git a/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs b/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs index 961344c2c..6629050d1 100644 --- a/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs +++ b/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs @@ -630,8 +630,8 @@ private static JsonElement AddMcpHeaderExtensions(JsonElement inputSchema, Metho if (!IsPrimitiveHeaderType(paramType)) { throw new InvalidOperationException( - $"Parameter '{param.Name}' on method '{method.Name}' has [McpHeader] but is not a primitive type. " + - "Only string, numeric, and boolean types may be annotated with [McpHeader]."); + $"Parameter '{param.Name}' on method '{method.Name}' has [McpHeader] but is not a supported type. " + + "Only string, integer, and boolean types may be annotated with [McpHeader]."); } // Validate case-insensitive uniqueness @@ -682,9 +682,6 @@ private static bool IsPrimitiveHeaderType(Type type) type == typeof(int) || type == typeof(uint) || type == typeof(long) || - type == typeof(ulong) || - type == typeof(float) || - type == typeof(double) || - type == typeof(decimal); + type == typeof(ulong); } } \ No newline at end of file diff --git a/src/ModelContextProtocol.Core/Server/McpHeaderAttribute.cs b/src/ModelContextProtocol.Core/Server/McpHeaderAttribute.cs index 516b73580..d81523a56 100644 --- a/src/ModelContextProtocol.Core/Server/McpHeaderAttribute.cs +++ b/src/ModelContextProtocol.Core/Server/McpHeaderAttribute.cs @@ -1,3 +1,5 @@ +using ModelContextProtocol.Client; + namespace ModelContextProtocol.Server; /// @@ -10,8 +12,8 @@ namespace ModelContextProtocol.Server; /// HTTP header named Mcp-Param-{Name}. /// /// -/// Only parameters with primitive types (string, number, boolean) may use this attribute. -/// The header name must contain only ASCII characters (0x21-0x7E, excluding space and colon) +/// Only parameters with primitive types (integer, string, boolean) may use this attribute. +/// The header name must match HTTP field-name token syntax (tchar per RFC 9110 Section 5.6.2) /// and must be case-insensitively unique within the tool's input schema. /// /// @@ -38,7 +40,7 @@ public sealed class McpHeaderAttribute : Attribute /// /// /// The name portion of the header. The full header name will be Mcp-Param-{name}. - /// Must contain only ASCII characters (0x21-0x7E, excluding space and colon). + /// Must match HTTP field-name token syntax (tchar per RFC 9110 Section 5.6.2). /// /// /// The name is null, empty, or contains invalid characters. @@ -59,23 +61,20 @@ public McpHeaderAttribute(string name) public string Name { get; } /// - /// Validates that a header name contains only valid characters. + /// Validates that a header name contains only valid HTTP token characters (tchar) per RFC 9110 Section 5.6.2. /// /// The header name to validate. /// The name contains invalid characters. internal static void ValidateHeaderName(string name) { - foreach (char c in name) + int idx = McpHeaderExtractor.FindFirstNonTchar(name); + if (idx >= 0) { - // Valid token characters per RFC 9110: visible ASCII (0x21-0x7E) excluding delimiters. - // Space (0x20) and colon (':') are explicitly prohibited. - if (c < 0x21 || c > 0x7E || c == ':') - { - throw new ArgumentException( - $"Header name contains invalid character '{c}' (0x{(int)c:X2}). " + - "Only ASCII characters (0x21-0x7E) excluding colon are allowed.", - nameof(name)); - } + char c = name[idx]; + throw new ArgumentException( + $"Header name contains invalid character '{c}' (0x{(int)c:X2}). " + + "Only HTTP token characters (tchar per RFC 9110 Section 5.6.2) are allowed.", + nameof(name)); } } } diff --git a/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs b/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs index 51db214ea..2346ab266 100644 --- a/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs @@ -106,10 +106,12 @@ public void DecodeValue_ValidBase64_Decodes() } [Fact] - public void DecodeValue_CaseInsensitivePrefix_Decodes() + public void DecodeValue_CaseSensitivePrefix_ReturnsLiteralValue() { + // Per SEP-2243: sentinel markers are case-sensitive and MUST appear exactly as shown (lowercase). + // An uppercase prefix should NOT be decoded as base64. var result = McpHeaderEncoder.DecodeValue("=?BASE64?SGVsbG8=?="); - Assert.Equal("Hello", result); + Assert.Equal("=?BASE64?SGVsbG8=?=", result); } [Fact] @@ -160,4 +162,34 @@ public void EncodeValue_EmbeddedTab_Base64Encodes() var decoded = McpHeaderEncoder.DecodeValue(result); Assert.Equal("col1\tcol2", decoded); } + + [Theory] + [InlineData("=?base64?literal?=")] + [InlineData("=?base64?SGVsbG8=?=")] + [InlineData("=?base64??=")] + public void EncodeValue_SentinelCollision_Base64Encodes(string input) + { + var result = McpHeaderEncoder.EncodeValue(input); + Assert.NotNull(result); + Assert.StartsWith("=?base64?", result); + Assert.EndsWith("?=", result); + + // The encoded value must be different from the input to avoid ambiguity + Assert.NotEqual(input, result); + + // Verify round-trip: decode must recover the original literal value + var decoded = McpHeaderEncoder.DecodeValue(result); + Assert.Equal(input, decoded); + } + + [Theory] + [InlineData("=?BASE64?literal?=", false)] // Case-sensitive: uppercase prefix does not match sentinel + [InlineData("=?base64?start", false)] // Missing suffix: no sentinel match + [InlineData("end?=", false)] // Missing prefix: no sentinel match + [InlineData("plain-text", false)] // No sentinel pattern + public void EncodeValue_NonSentinelPattern_NotBase64Encoded(string input, bool _) + { + var result = McpHeaderEncoder.EncodeValue(input); + Assert.Equal(input, result); + } } diff --git a/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs b/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs new file mode 100644 index 000000000..eec71da0e --- /dev/null +++ b/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs @@ -0,0 +1,152 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using System.Text.Json; + +namespace ModelContextProtocol.Tests.Client; + +/// +/// Tests for SEP-2243 x-mcp-header validation changes: +/// - RFC 9110 tchar validation for header names +/// - "number" type rejection (only integer/string/boolean allowed) +/// - Nested property support for x-mcp-header annotations +/// +public class McpHeaderExtractorValidationTests : ClientServerTestBase +{ + public McpHeaderExtractorValidationTests(ITestOutputHelper outputHelper) + : base(outputHelper) + { + } + + protected override void ConfigureServices(ServiceCollection services, IMcpServerBuilder mcpServerBuilder) + { + // Valid baseline tool + mcpServerBuilder.WithTools([McpServerTool.Create( + (string input) => $"echo {input}", + new() { Name = "ValidTool" })]); + + // Tool with "number" type (should be rejected per updated SEP-2243) + var numberTool = McpServerTool.Create((string x) => x, new() { Name = "NumberTypeTool" }); + numberTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "value": { "type": "number", "x-mcp-header": "Value" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([numberTool]); + + // Tool with "integer" type (should be accepted) + var integerTool = McpServerTool.Create((string x) => x, new() { Name = "IntegerTypeTool" }); + integerTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "count": { "type": "integer", "x-mcp-header": "Count" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([integerTool]); + + // Tool with non-tchar header name (should be rejected) + var nonTcharTool = McpServerTool.Create((string x) => x, new() { Name = "BadTcharTool" }); + nonTcharTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "region": { "type": "string", "x-mcp-header": "Region(1)" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([nonTcharTool]); + + // Tool with valid nested x-mcp-header + var nestedValidTool = McpServerTool.Create((string x) => x, new() { Name = "NestedValidTool" }); + nestedValidTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "config": { "type": "object", "properties": { "region": { "type": "string", "x-mcp-header": "Region" } } } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([nestedValidTool]); + + // Tool with invalid nested x-mcp-header (colon in header name) + var nestedInvalidTool = McpServerTool.Create((string x) => x, new() { Name = "NestedInvalidTool" }); + nestedInvalidTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "config": { "type": "object", "properties": { "region": { "type": "string", "x-mcp-header": "Invalid:Header" } } } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([nestedInvalidTool]); + + // Tool with duplicate header names across nesting levels + var duplicateTool = McpServerTool.Create((string x) => x, new() { Name = "DuplicateHeaderTool" }); + duplicateTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "topRegion": { "type": "string", "x-mcp-header": "Region" }, "nested": { "type": "object", "properties": { "innerRegion": { "type": "string", "x-mcp-header": "region" } } } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([duplicateTool]); + + // Tool with nested "number" type (should be rejected) + var nestedNumberTool = McpServerTool.Create((string x) => x, new() { Name = "NestedNumberTool" }); + nestedNumberTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "config": { "type": "object", "properties": { "threshold": { "type": "number", "x-mcp-header": "Threshold" } } } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([nestedNumberTool]); + } + + [Fact] + public async Task ListToolsAsync_NumberType_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "NumberTypeTool"); + + Assert.Contains(MockLoggerProvider.LogMessages, log => + log.LogLevel == LogLevel.Warning && + log.Message.Contains("NumberTypeTool") && + log.Message.Contains("excluded")); + } + + [Fact] + public async Task ListToolsAsync_IntegerType_AcceptsTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "IntegerTypeTool"); + } + + [Fact] + public async Task ListToolsAsync_NonTcharHeaderName_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "BadTcharTool"); + } + + [Fact] + public async Task ListToolsAsync_NestedValidHeader_AcceptsTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "NestedValidTool"); + } + + [Fact] + public async Task ListToolsAsync_NestedInvalidHeader_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "NestedInvalidTool"); + } + + [Fact] + public async Task ListToolsAsync_NestedDuplicateHeaders_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "DuplicateHeaderTool"); + } + + [Fact] + public async Task ListToolsAsync_NestedNumberType_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "NestedNumberTool"); + } +} diff --git a/tests/ModelContextProtocol.Tests/Server/McpHeaderAttributeTests.cs b/tests/ModelContextProtocol.Tests/Server/McpHeaderAttributeTests.cs index 694869af9..85374a589 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpHeaderAttributeTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpHeaderAttributeTests.cs @@ -9,6 +9,9 @@ public class McpHeaderAttributeTests [InlineData("TenantId")] [InlineData("Priority")] [InlineData("X-Custom")] + [InlineData("x!header")] + [InlineData("x#header")] + [InlineData("x~header")] public void Constructor_ValidHeaderName_Succeeds(string name) { var attr = new McpHeaderAttribute(name); @@ -27,6 +30,23 @@ public void Constructor_NameWithColon_Throws() Assert.Throws(() => new McpHeaderAttribute("Region:Primary")); } + [Theory] + [InlineData("Region(1)")] + [InlineData("path/to")] + [InlineData("key=value")] + [InlineData("name@host")] + [InlineData("with,comma")] + [InlineData("with;semi")] + [InlineData("with[bracket")] + [InlineData("with{brace")] + [InlineData("with\"quote")] + [InlineData("with\\backslash")] + [InlineData("with?question")] + public void Constructor_NonTcharCharacter_Throws(string name) + { + Assert.Throws(() => new McpHeaderAttribute(name)); + } + [Fact] public void Constructor_NullName_Throws() { From f7a15fb6cd409c84612e4dc62ac7db816f20cda4 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 2 Jun 2026 12:31:58 -0700 Subject: [PATCH 2/2] Address review feedback on x-mcp-header validation - Reject x-mcp-header parameters whose JSON Schema type is a disallowed or malformed value, including non-string/union types. A union array is accepted only when it contains at least one of string/integer/boolean (with optional "null"); an absent type is still treated as unknown and allowed to avoid excluding valid enum/const/$ref schemas. - Reword the rejection message: "number" is a JSON primitive but is not a permitted header type, so report it as an unsupported type rather than non-primitive. - Clarify the ListToolsAsync comment to note that the client validates x-mcp-header annotations on all transports, while only Streamable HTTP is required to by SEP-2243. - Drop the unused parameter from EncodeValue_NonSentinelPattern_NotBase64Encoded. - Add tests for nullable union, disallowed union, null-only union, and missing-type schemas. --- .../Client/McpClient.Methods.cs | 7 +- .../Client/McpHeaderExtractor.cs | 67 ++++++++++++++++--- .../Client/McpHeaderEncoderTests.cs | 10 +-- .../McpHeaderExtractorValidationTests.cs | 66 ++++++++++++++++++ 4 files changed, 133 insertions(+), 17 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs b/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs index 59ee6edc2..79d4b0a02 100644 --- a/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs +++ b/src/ModelContextProtocol.Core/Client/McpClient.Methods.cs @@ -186,9 +186,10 @@ public async ValueTask> ListToolsAsync( tools ??= new(toolResults.Tools.Count); foreach (var tool in toolResults.Tools) { - // Validate x-mcp-header annotations per SEP-2243. - // Streamable HTTP clients MUST exclude tools with invalid annotations; - // non-HTTP clients MAY also reject them for safety. + // Validate x-mcp-header annotations per SEP-2243. The spec requires Streamable HTTP + // clients to exclude tools with invalid annotations and permits other transports + // (e.g., stdio) to ignore the annotations entirely. This client validates on all + // transports so a malformed definition is rejected consistently regardless of transport. if (!McpHeaderExtractor.ValidateToolSchema(tool, out var rejectionReason)) { ToolRejected?.Invoke(tool, rejectionReason!); diff --git a/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs b/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs index 2206a98cc..a339c5b58 100644 --- a/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs +++ b/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs @@ -180,23 +180,72 @@ private static bool ValidateProperties(Tool tool, JsonElement properties, HashSe return false; } - // MUST only be applied to primitive types (integer, string, boolean). - // Parameters with type "number" are not permitted. + // MUST only be applied to parameters with primitive types (string, integer, boolean). + // Parameters with type "number" (or any other non-primitive type) are not permitted. + // The "type" keyword may be omitted (treated as unknown, not rejected, since many valid + // schemas constrain the value via enum/const/$ref instead) or expressed as a JSON Schema + // union array such as ["string", "null"]; only an explicitly disallowed or malformed type + // causes rejection. if (property.Value.TryGetProperty("type", out var typeElement) && - typeElement.ValueKind == JsonValueKind.String) + !IsAllowedHeaderType(typeElement)) { - var typeName = typeElement.GetString(); - if (typeName is not ("string" or "integer" or "boolean")) - { - rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' has non-primitive type '{typeName}'."; - return false; - } + rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' has unsupported type '{typeElement}'. Only 'string', 'integer', and 'boolean' are allowed."; + return false; } } return true; } + /// + /// Determines whether a JSON Schema type keyword is compatible with x-mcp-header, + /// which per SEP-2243 may only be applied to string, integer, or boolean + /// parameters. A union array (e.g., ["string", "null"]) is allowed as long as it contains + /// at least one allowed primitive; "null" is tolerated only as an additional union member. + /// Any other shape (a disallowed type name, a non-string array element, an empty array, or a + /// non-string/non-array value) is treated as incompatible. + /// + private static bool IsAllowedHeaderType(JsonElement typeElement) + { + switch (typeElement.ValueKind) + { + case JsonValueKind.String: + return IsAllowedPrimitiveTypeName(typeElement.GetString()); + + case JsonValueKind.Array: + bool hasAllowedPrimitive = false; + foreach (var entry in typeElement.EnumerateArray()) + { + if (entry.ValueKind != JsonValueKind.String) + { + return false; + } + + var entryName = entry.GetString(); + if (entryName == "null") + { + continue; + } + + if (!IsAllowedPrimitiveTypeName(entryName)) + { + return false; + } + + hasAllowedPrimitive = true; + } + + return hasAllowedPrimitive; + + default: + // A "type" that is present but is neither a string nor an array of strings is malformed. + return false; + } + } + + private static bool IsAllowedPrimitiveTypeName(string? typeName) => + typeName is "string" or "integer" or "boolean"; + // Valid HTTP token characters (tchar) per RFC 9110 Section 5.6.2: // tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / // "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA diff --git a/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs b/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs index 2346ab266..65c0b31d9 100644 --- a/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/McpHeaderEncoderTests.cs @@ -183,11 +183,11 @@ public void EncodeValue_SentinelCollision_Base64Encodes(string input) } [Theory] - [InlineData("=?BASE64?literal?=", false)] // Case-sensitive: uppercase prefix does not match sentinel - [InlineData("=?base64?start", false)] // Missing suffix: no sentinel match - [InlineData("end?=", false)] // Missing prefix: no sentinel match - [InlineData("plain-text", false)] // No sentinel pattern - public void EncodeValue_NonSentinelPattern_NotBase64Encoded(string input, bool _) + [InlineData("=?BASE64?literal?=")] // Case-sensitive: uppercase prefix does not match sentinel + [InlineData("=?base64?start")] // Missing suffix: no sentinel match + [InlineData("end?=")] // Missing prefix: no sentinel match + [InlineData("plain-text")] // No sentinel pattern + public void EncodeValue_NonSentinelPattern_NotBase64Encoded(string input) { var result = McpHeaderEncoder.EncodeValue(input); Assert.Equal(input, result); diff --git a/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs b/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs index eec71da0e..ff3916d2a 100644 --- a/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs @@ -75,6 +75,34 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer { "type": "object", "properties": { "config": { "type": "object", "properties": { "threshold": { "type": "number", "x-mcp-header": "Threshold" } } } } } """).RootElement.Clone(); mcpServerBuilder.WithTools([nestedNumberTool]); + + // Tool with a nullable union type ["string", "null"] (should be accepted) + var nullableUnionTool = McpServerTool.Create((string x) => x, new() { Name = "NullableUnionTool" }); + nullableUnionTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "region": { "type": ["string", "null"], "x-mcp-header": "Region" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([nullableUnionTool]); + + // Tool with a union type containing a disallowed type ["number", "null"] (should be rejected) + var numberUnionTool = McpServerTool.Create((string x) => x, new() { Name = "NumberUnionTool" }); + numberUnionTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "value": { "type": ["number", "null"], "x-mcp-header": "Value" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([numberUnionTool]); + + // Tool with a "null"-only union type (should be rejected: no allowed primitive present) + var nullOnlyTool = McpServerTool.Create((string x) => x, new() { Name = "NullOnlyTool" }); + nullOnlyTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "value": { "type": ["null"], "x-mcp-header": "Value" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([nullOnlyTool]); + + // Tool whose annotated property omits "type" (should be accepted: type is unknown, not invalid) + var missingTypeTool = McpServerTool.Create((string x) => x, new() { Name = "MissingTypeTool" }); + missingTypeTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "region": { "x-mcp-header": "Region" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([missingTypeTool]); } [Fact] @@ -149,4 +177,42 @@ public async Task ListToolsAsync_NestedNumberType_ExcludesTool() Assert.Contains(tools, t => t.Name == "ValidTool"); Assert.DoesNotContain(tools, t => t.Name == "NestedNumberTool"); } + + [Fact] + public async Task ListToolsAsync_NullableUnionType_AcceptsTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "NullableUnionTool"); + } + + [Fact] + public async Task ListToolsAsync_NumberUnionType_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "NumberUnionTool"); + } + + [Fact] + public async Task ListToolsAsync_NullOnlyUnionType_ExcludesTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "ValidTool"); + Assert.DoesNotContain(tools, t => t.Name == "NullOnlyTool"); + } + + [Fact] + public async Task ListToolsAsync_MissingType_AcceptsTool() + { + await using var client = await CreateMcpClientForServer(); + var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains(tools, t => t.Name == "MissingTypeTool"); + } }