From f3486fed0839bab9065f4e7835fd4168da4527d7 Mon Sep 17 00:00:00 2001 From: 2sumtech <2sumtech@gmail.com> Date: Sat, 26 Sep 2026 14:09:30 -0700 Subject: [PATCH] fix(server): answer malformed params for spec methods with invalid params A request for a spec method whose params fail to deserialize (for example `tools/call` without `name`, or with non-object `arguments`) falls back to `ClientRequest::CustomRequest`. The default `on_custom_request` then answered -32601 Method not found, which on modern Streamable HTTP is also sent as HTTP 404. The method exists, so answer -32602 Invalid params, as the tools spec shows for requests that fail the CallToolRequest schema. Only methods dispatched without an era or capability gate are mapped; initialize, server/discover, ping, subscriptions and tasks/* keep -32601. Co-Authored-By: Claude Fable 5.1 --- crates/rmcp/src/handler/server.rs | 37 ++++- .../tests/test_malformed_request_params.rs | 126 ++++++++++++++++++ 2 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 crates/rmcp/tests/test_malformed_request_params.rs diff --git a/crates/rmcp/src/handler/server.rs b/crates/rmcp/src/handler/server.rs index f84672451..6c7d07917 100644 --- a/crates/rmcp/src/handler/server.rs +++ b/crates/rmcp/src/handler/server.rs @@ -47,6 +47,25 @@ fn validate_tasks_capability( } } +/// Client request methods that the server dispatches without a protocol-era +/// or capability gate. `initialize`, `server/discover`, `ping`, the +/// subscription methods and `tasks/*` are left out: for those a -32601 answer +/// can be correct (era or capability mismatch) and drives client fallback. +fn is_ungated_spec_request_method(method: &str) -> bool { + [ + CallToolRequestMethod::VALUE, + ListToolsRequestMethod::VALUE, + GetPromptRequestMethod::VALUE, + ListPromptsRequestMethod::VALUE, + ListResourcesRequestMethod::VALUE, + ListResourceTemplatesRequestMethod::VALUE, + ReadResourceRequestMethod::VALUE, + CompleteRequestMethod::VALUE, + SetLevelRequestMethod::VALUE, + ] + .contains(&method) +} + impl Service for H { async fn handle_request( &self, @@ -220,10 +239,20 @@ impl Service for H { .list_tools(request.params, context) .await .map(ServerResult::ListToolsResult), - ClientRequest::CustomRequest(request) => self - .on_custom_request(request, context) - .await - .map(ServerResult::CustomResult), + ClientRequest::CustomRequest(request) => { + // A spec method only reaches `CustomRequest` when its params + // failed to deserialize into the typed request. The method + // exists, so the error is -32602 Invalid params, not -32601. + if is_ungated_spec_request_method(&request.method) { + return Err(McpError::invalid_params( + format!("invalid params for {}", request.method), + None, + )); + } + self.on_custom_request(request, context) + .await + .map(ServerResult::CustomResult) + } ClientRequest::GetTaskRequest(request) => { validate_tasks_capability::(self, &context)?; self.get_task(request.params, context) diff --git a/crates/rmcp/tests/test_malformed_request_params.rs b/crates/rmcp/tests/test_malformed_request_params.rs new file mode 100644 index 000000000..ca47171b2 --- /dev/null +++ b/crates/rmcp/tests/test_malformed_request_params.rs @@ -0,0 +1,126 @@ +//! A request for a spec method whose params do not match the schema falls back +//! to `ClientRequest::CustomRequest` during deserialization. The server must +//! answer it with -32602 Invalid params: the method exists, so -32601 Method not +//! found is wrong (and on modern Streamable HTTP it also becomes a 404). +#![cfg(all(feature = "server", not(feature = "local")))] + +use std::time::Duration; + +use rmcp::{ + ErrorData as McpError, RoleServer, ServerHandler, ServiceExt, + model::{ + CallToolRequestParams, CallToolResponse, CallToolResult, ContentBlock, ServerCapabilities, + ServerConfig, + }, + service::RequestContext, +}; +use serde_json::{Value, json}; +use tokio::io::{ + AsyncBufReadExt, AsyncWrite, AsyncWriteExt, BufReader, DuplexStream, Lines, ReadHalf, +}; + +#[derive(Debug, Clone, Default)] +struct ToolServer; + +impl ServerHandler for ToolServer { + fn get_info(&self) -> ServerConfig { + ServerConfig::new(ServerCapabilities::builder().enable_tools().build()) + } + + async fn call_tool( + &self, + _request: CallToolRequestParams, + _context: RequestContext, + ) -> Result { + Ok(CallToolResult::success(vec![ContentBlock::text("ok")]).into()) + } +} + +async fn send(write: &mut (impl AsyncWrite + Unpin), message: Value) -> anyhow::Result<()> { + let mut line = serde_json::to_vec(&message)?; + line.push(b'\n'); + write.write_all(&line).await?; + Ok(()) +} + +async fn recv(lines: &mut Lines>>) -> anyhow::Result { + let line = tokio::time::timeout(Duration::from_secs(5), lines.next_line()) + .await?? + .ok_or_else(|| anyhow::anyhow!("server closed the stream"))?; + Ok(serde_json::from_str(&line)?) +} + +#[tokio::test] +async fn malformed_params_for_spec_method_is_invalid_params() -> anyhow::Result<()> { + let (server_io, client_io) = tokio::io::duplex(4096); + let server = tokio::spawn(async move { + let running = ToolServer.serve(server_io).await?; + running.waiting().await?; + anyhow::Ok(()) + }); + let (read, mut write) = tokio::io::split(client_io); + let mut lines = BufReader::new(read).lines(); + + send( + &mut write, + json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": { "name": "raw-client", "version": "0.0.0" } + } + }), + ) + .await?; + assert_eq!(recv(&mut lines).await?["id"], 1); + send( + &mut write, + json!({ "jsonrpc": "2.0", "method": "notifications/initialized" }), + ) + .await?; + + // `name` is required by CallToolRequest; `arguments` must be an object. + for (id, params) in [ + (2, json!({ "arguments": {} })), + (3, json!({ "name": "echo", "arguments": "not-an-object" })), + ] { + send( + &mut write, + json!({ "jsonrpc": "2.0", "id": id, "method": "tools/call", "params": params }), + ) + .await?; + let response = recv(&mut lines).await?; + assert_eq!(response["id"], id, "{response}"); + assert_eq!(response["error"]["code"], -32602, "{response}"); + } + + // The tool itself works, and an unknown method is still -32601. + send( + &mut write, + json!({ + "jsonrpc": "2.0", "id": 4, "method": "tools/call", + "params": { "name": "echo", "arguments": {} } + }), + ) + .await?; + let response = recv(&mut lines).await?; + assert_eq!(response["id"], 4, "{response}"); + assert!(response.get("result").is_some(), "{response}"); + + send( + &mut write, + json!({ "jsonrpc": "2.0", "id": 5, "method": "no/such/method" }), + ) + .await?; + let response = recv(&mut lines).await?; + assert_eq!(response["id"], 5, "{response}"); + assert_eq!(response["error"]["code"], -32601, "{response}"); + + drop(write); + drop(lines); + server.await??; + Ok(()) +}