From 6af4fbe38b797fa389afe7bce7b2874979d685f0 Mon Sep 17 00:00:00 2001 From: halter73 via Copilot <223556219+Copilot@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:50:39 -0700 Subject: [PATCH] Accept "type":"number" as a valid x-mcp-header primitive per SEP-2243 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the SEP-2243 spec text (specification/draft/server/tools.mdx, line 340): > MUST only be applied to parameters with primitive types (number, string, boolean) Our McpHeaderExtractor.ValidateToolSchema was rejecting tools whose x-mcp-header annotation targeted a property with "type":"number", contradicting the spec and causing the upstream conformance scenario `http-custom-headers` to fail (the schema declares `priority` and `float_val` as "type":"number" and expects them to be emitted as Mcp-Param-Priority / Mcp-Param-FloatVal headers). McpHeaderEncoder.ConvertToHeaderValue already handles JsonValueKind.Number by emitting element.GetRawText(), so number-typed values flow through the encoder unchanged — only the validator needed updating. Integer canonicalization (per the SEP's JavaScript safe integer rule) remains restricted to "type":"integer"; "type":"number" values are emitted using their raw JSON representation (e.g. 2.5, 3.14, -7), which is what the conformance suite expects. Updates: - Add "number" to IsAllowedPrimitiveTypeName. - Refresh doc comments + rejection message to match the spec wording. - Flip three previously-asserted-as-rejected test cases in McpHeaderExtractorValidationTests to assert acceptance, and add an ArrayTypeTool to retain a negative-case coverage point. - Add CallTool_NumberType_EmitsRawJsonNumberHeader in AddKnownToolsHeaderTests for end-to-end wire coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Client/McpHeaderExtractor.cs | 23 ++++--- .../AddKnownToolsHeaderTests.cs | 68 +++++++++++++++++++ .../McpHeaderExtractorValidationTests.cs | 50 +++++++++----- 3 files changed, 113 insertions(+), 28 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs b/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs index 99fe5462a..1be48bd6d 100644 --- a/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs +++ b/src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs @@ -267,16 +267,16 @@ private static bool ValidateProperties(Tool tool, JsonElement properties, HashSe return false; } - // 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 + // MUST only be applied to parameters with primitive types (number, string, boolean) per + // SEP-2243. We also accept "integer" as a JSON Schema refinement of "number". 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) && !IsAllowedHeaderType(typeElement)) { - rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' has unsupported type '{typeElement}'. Only 'string', 'integer', and 'boolean' are allowed."; + rejectionReason = $"Tool '{tool.Name}': x-mcp-header on property '{property.Name}' has unsupported type '{typeElement}'. Only 'string', 'integer', 'number', and 'boolean' are allowed."; return false; } } @@ -286,10 +286,11 @@ private static bool ValidateProperties(Tool tool, JsonElement properties, HashSe /// /// 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 + /// which per SEP-2243 may only be applied to number, string, or boolean + /// parameters. We additionally accept integer as a JSON Schema refinement of number. + /// 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) @@ -331,7 +332,7 @@ private static bool IsAllowedHeaderType(JsonElement typeElement) } private static bool IsAllowedPrimitiveTypeName(string? typeName) => - typeName is "string" or "integer" or "boolean"; + typeName is "string" or "integer" or "number" or "boolean"; // Valid HTTP token characters (tchar) per RFC 9110 Section 5.6.2: // tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/AddKnownToolsHeaderTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/AddKnownToolsHeaderTests.cs index 852fb122e..fc1b34d5c 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/AddKnownToolsHeaderTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/AddKnownToolsHeaderTests.cs @@ -342,6 +342,74 @@ public async Task RemoveKnownTools_ThenCallTool_NoMcpParamHeaders() Assert.Empty(headers); } + private static Tool CreateToolWithNumberHeaders() + { + // Schema using "type": "number" for both an integer-valued and a fractional-valued + // header parameter. Per SEP-2243 the "number" primitive type is permitted alongside + // "string" and "boolean"; unlike "integer", values aren't canonicalized — they are + // emitted using their raw JSON representation. + var schemaJson = """ + { + "type": "object", + "properties": { + "priority": { + "type": "number", + "x-mcp-header": "Priority" + }, + "ratio": { + "type": "number", + "x-mcp-header": "Ratio" + } + }, + "required": ["priority", "ratio"] + } + """; + + return new Tool + { + Name = "number_tool", + InputSchema = JsonDocument.Parse(schemaJson).RootElement.Clone(), + }; + } + + [Theory] + [InlineData("2", "0.5", "2", "0.5")] + [InlineData("42", "3.14", "42", "3.14")] + [InlineData("-7", "-0.25", "-7", "-0.25")] + public async Task CallTool_NumberType_EmitsRawJsonNumberHeader( + string priorityValue, + string ratioValue, + string expectedPriorityHeader, + string expectedRatioHeader) + { + await StartAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new("http://localhost:5000/mcp"), + TransportMode = HttpTransportMode.StreamableHttp, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, + cancellationToken: TestContext.Current.CancellationToken); + + client.AddKnownTools([CreateToolWithNumberHeaders()]); + + var result = await client.CallToolAsync( + "number_tool", + new Dictionary + { + ["priority"] = JsonDocument.Parse(priorityValue).RootElement, + ["ratio"] = JsonDocument.Parse(ratioValue).RootElement, + }, + cancellationToken: TestContext.Current.CancellationToken); + + Assert.NotNull(result); + var headers = _capturedHeaders.Values.First(); + Assert.Equal(expectedPriorityHeader, headers["Mcp-Param-Priority"]); + Assert.Equal(expectedRatioHeader, headers["Mcp-Param-Ratio"]); + } + private static Tool CreateToolWithSingleHeader(string toolName, string headerName) { var schemaJson = $$""" diff --git a/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs b/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs index ff3916d2a..14ee44e66 100644 --- a/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs +++ b/tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs @@ -10,7 +10,8 @@ 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) +/// - "number" type acceptance (along with integer/string/boolean) per the SEP's +/// "primitive types (number, string, boolean)" rule /// - Nested property support for x-mcp-header annotations /// public class McpHeaderExtractorValidationTests : ClientServerTestBase @@ -27,7 +28,7 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer (string input) => $"echo {input}", new() { Name = "ValidTool" })]); - // Tool with "number" type (should be rejected per updated SEP-2243) + // Tool with "number" type (should be accepted per SEP-2243 "number, string, boolean" rule) 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" } } } @@ -41,6 +42,13 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer """).RootElement.Clone(); mcpServerBuilder.WithTools([integerTool]); + // Tool with "array" type (should be rejected - not a primitive type) + var arrayTool = McpServerTool.Create((string x) => x, new() { Name = "ArrayTypeTool" }); + arrayTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" + { "type": "object", "properties": { "value": { "type": "array", "items": { "type": "string" }, "x-mcp-header": "Value" } } } + """).RootElement.Clone(); + mcpServerBuilder.WithTools([arrayTool]); + // Tool with non-tchar header name (should be rejected) var nonTcharTool = McpServerTool.Create((string x) => x, new() { Name = "BadTcharTool" }); nonTcharTool.ProtocolTool.InputSchema = JsonDocument.Parse(""" @@ -69,7 +77,7 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer """).RootElement.Clone(); mcpServerBuilder.WithTools([duplicateTool]); - // Tool with nested "number" type (should be rejected) + // Tool with nested "number" type (should be accepted per SEP-2243) 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" } } } } } @@ -83,7 +91,7 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer """).RootElement.Clone(); mcpServerBuilder.WithTools([nullableUnionTool]); - // Tool with a union type containing a disallowed type ["number", "null"] (should be rejected) + // Tool with a union type containing "number" and "null" (should be accepted) 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" } } } @@ -106,18 +114,13 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer } [Fact] - public async Task ListToolsAsync_NumberType_ExcludesTool() + public async Task ListToolsAsync_NumberType_AcceptsTool() { 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")); + Assert.Contains(tools, t => t.Name == "NumberTypeTool"); } [Fact] @@ -129,6 +132,21 @@ public async Task ListToolsAsync_IntegerType_AcceptsTool() Assert.Contains(tools, t => t.Name == "IntegerTypeTool"); } + [Fact] + public async Task ListToolsAsync_ArrayType_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 == "ArrayTypeTool"); + + Assert.Contains(MockLoggerProvider.LogMessages, log => + log.LogLevel == LogLevel.Warning && + log.Message.Contains("ArrayTypeTool") && + log.Message.Contains("excluded")); + } + [Fact] public async Task ListToolsAsync_NonTcharHeaderName_ExcludesTool() { @@ -169,13 +187,12 @@ public async Task ListToolsAsync_NestedDuplicateHeaders_ExcludesTool() } [Fact] - public async Task ListToolsAsync_NestedNumberType_ExcludesTool() + public async Task ListToolsAsync_NestedNumberType_AcceptsTool() { 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"); + Assert.Contains(tools, t => t.Name == "NestedNumberTool"); } [Fact] @@ -188,13 +205,12 @@ public async Task ListToolsAsync_NullableUnionType_AcceptsTool() } [Fact] - public async Task ListToolsAsync_NumberUnionType_ExcludesTool() + public async Task ListToolsAsync_NumberUnionType_AcceptsTool() { 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"); + Assert.Contains(tools, t => t.Name == "NumberUnionTool"); } [Fact]