Skip to content

Match protocol property names case-sensitively - #1934

Open
FenjuFu wants to merge 1 commit into
modelcontextprotocol:mainfrom
FenjuFu:fix/protocol-property-casing
Open

FenjuFu wants to merge 1 commit into
modelcontextprotocol:mainfrom
FenjuFu:fix/protocol-property-casing

Conversation

@FenjuFu

@FenjuFu FenjuFu commented Oct 15, 2026

Copy link
Copy Markdown

Fixes #1897

The JSON-RPC envelope converter matches jsonrpc/id/method/params/result/error exactly, but the typed contents of params and result were deserialized with the internal source-generated JsonContext, which is declared with JsonSerializerDefaults.Web and therefore binds property names case-insensitively. A tools/call request with params.Name was dispatched as if it were params.name, even though middleware or proxies inspecting params.name would not recognize it.

Changes

  • McpJsonUtilities.JsonContext now sets PropertyNameCaseInsensitive = false. Protocol DTOs (requests, results and notifications, on both client and server) only bind the property names defined by the specification, consistent with the envelope.
  • McpJsonUtilities.DefaultOptions explicitly keeps PropertyNameCaseInsensitive = true. It is copied from the context's options but is also used for user-defined types (tool, prompt and resource parameters, McpClientTool arguments), so binding of user data is unchanged. The XML docs on DefaultOptions now describe this split.

As discussed in the issue, this is exact protocol-field matching, not new validation of non-canonical JSON:

tools/call params Before After
{"Name":"echo"} dispatched to echo name missing, JSON-RPC error, handler not invoked
{"name":"echo","Name":"other"} last value wins (other) echo
{"Name":"other","name":"echo"} echo echo
{"name":"echo","Arguments":{...}} arguments bound arguments not bound (null)
{"name":"echo","arguments":{"text":"a","Text":"B"}} both keys kept both keys kept (user data)

Fields with defaults keep their defaults when only an alternately cased key is present; for example a CallToolResult with only Content still ends up with an empty content list rather than failing.

Not changed

DefaultOptions.GetTypeInfo(<protocol type>) remains case-insensitive, because the setting applies per options instance rather than per type. Code that deserializes protocol types manually through DefaultOptions keeps the old behavior. Inside the SDK, this affects ModelContextProtocol.Extensions.Tasks, which deserializes CallToolResult, InputRequest and IDictionary<string, InputResponse> through DefaultOptions in six places (McpTasksClientExtensions, GetTaskResult, TaskStatusNotificationParams, McpTasksBuilderExtensions). I left them as they are to keep this change scoped to the core protocol path; happy to switch them to the source-generated type infos here or in a follow-up if you prefer.

Tests

  • McpServerTests.Call_Tool_Request_Params_Are_Matched_Case_Sensitively sends raw tools/call requests through the server: Name only (rejected, handler not invoked), alternately cased Arguments (not bound), canonical, both key orders with name and Name, and alternately cased keys inside arguments (preserved as distinct entries).
  • McpJsonUtilitiesTests.DefaultOptions_MatchesPropertyNamesCaseInsensitively checks that DefaultOptions still binds user types case-insensitively.

The new server test fails without the JsonContext change. Locally on net10.0 (Windows): ModelContextProtocol.Tests 2393 passed / 6 skipped, ModelContextProtocol.AspNetCore.Tests 635 passed / 30 skipped, 0 failed.

As noted in the issue, this is a subtle behavior change for peers that relied on alternate casing, so it probably belongs in a minor release.

cc @tarekgh for review

Protocol DTOs were deserialized with the source-generated context, which uses the web defaults and binds property names case-insensitively, while the JSON-RPC envelope converter matches its properties exactly. A request such as tools/call with params.Name was therefore dispatched even though middleware inspecting params.name would not recognize it.

The source-generated context now matches property names case-sensitively. DefaultOptions keeps case-insensitive matching because it is also used for user-defined types such as tool parameters.

Fixes modelcontextprotocol#1897

Signed-off-by: FenjuFu <fufenjupku@gmail.com>

@tarekgh tarekgh left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. This approach makes sense: match protocol-defined fields exactly while preserving the existing DefaultOptions behavior for user-defined data.

I checked TypeScript, Python, Go, Rust, and Ruby. None binds params.Name as the canonical tool name, and canonical name wins when both spellings are present, regardless of order. TypeScript and Python runtime probes confirmed this in both protocol eras. This supports the change here.

The important distinction is exact matching, not blanket rejection of noncanonical JSON. Peers differ in required-field validation, defaults, and raw/custom-result handling.

Looks good overall. A few non-blocking suggestions:

  • Add a client-side result test covering alternate-cased optional fields such as Content and IsError, and a nested user-DTO-through-tool test confirming that its existing binding behavior is preserved.
  • Track the remaining internal casing differences separately, particularly task-based CallToolResult deserialization and transport-level JsonRpcErrorDetail parsing. These do not bypass the tool-name fix.
  • Include the behavior change in the planned minor-release notes. The shared context also changes OAuth model parsing, so that compatibility impact should be mentioned.

CC @jeffhandley @halter73

/// </para>
/// <para>
/// Property names are matched case-insensitively, as with <see cref="JsonSerializerDefaults.Web"/>. The SDK itself
/// deserializes incoming protocol messages with case-sensitive contracts, so a protocol property such as

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The SDK itself deserializes incoming protocol messages with case-sensitive contracts" is broader than the implementation. Some internal readers still use the case-insensitive public options, including task-based tool-result readers and transport-level error-detail parsing.

Could we qualify this wording and explicitly document that protocol types deserialized through DefaultOptions retain case-insensitive matching? That would describe the boundary accurately without requiring those follow-ups to block this fix.

@halter73

Copy link
Copy Markdown
Contributor

I'd rather finish the protocol/user-data split here than leave the remaining readers as follow-ups. Tasks tool results still bind IsError, and stream/ASP.NET error readers still bind Code/Message.

We could add cached, case-sensitive McpJsonUtilities.ProtocolOptions for protocol deserialization throughout the SDK, keeping DefaultOptions and caller-supplied options for user-defined data. If we want to avoid new public API, a shared internal helper under src/Common would work too, but a public preset would also be useful to consumers deserializing protocol messages themselves.

@tarekgh I know you suggested tracking these separately, but fixing the remaining protocol-field casing inconsistencies together seems manageable in one PR. Does that sound good to you?

@tarekgh

tarekgh commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

@halter73 Yes, that sounds good to me. My suggestion to track these separately was to keep the original fix focused, but I agree that finishing the protocol/user-data split in this PR is cleaner.

A cached, read-only McpJsonUtilities.ProtocolOptions preset seems useful both internally and for consumers deserializing protocol messages themselves. It would also give extension packages a consistent contract without needing access to the internal context.

Let's use it for the remaining protocol readers, including task-based tool results and stream/ASP.NET error details, while retaining DefaultOptions and caller-supplied options for user-defined data. We should preserve source-generated/Native AOT support and add regression coverage for those paths, along with confirmation that nested user-data binding remains unchanged.

That would let us document the protocol boundary consistently rather than carrying the known internal exceptions.

@FenjuFu Would you be able to include these changes in this PR, or would you like help with the implementation or tests?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON property-name casing is inconsistent between JSON-RPC envelope and request params

3 participants