From 3197be77cf166b17ffbd6c66641b20a35a4d4185 Mon Sep 17 00:00:00 2001 From: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com> Date: Thu, 24 Sep 2026 13:52:50 -0400 Subject: [PATCH] style: cargo fmt fixes on validate_standard_headers changes Signed-off-by: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com> Signed-off-by: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com> --- .../transport/streamable_http_server/tower.rs | 138 +++++++++++++++++- .../test_streamable_http_standard_headers.rs | 86 +++++++++++ 2 files changed, 220 insertions(+), 4 deletions(-) diff --git a/crates/rmcp/src/transport/streamable_http_server/tower.rs b/crates/rmcp/src/transport/streamable_http_server/tower.rs index 1b5382015..0f9b8bece 100644 --- a/crates/rmcp/src/transport/streamable_http_server/tower.rs +++ b/crates/rmcp/src/transport/streamable_http_server/tower.rs @@ -39,8 +39,8 @@ use crate::{ OneshotTransport, TransportAdapterIdentity, common::{ http_header::{ - EVENT_STREAM_MIME_TYPE, HEADER_LAST_EVENT_ID, HEADER_MCP_PROTOCOL_VERSION, - HEADER_SESSION_ID, JSON_MIME_TYPE, + EVENT_STREAM_MIME_TYPE, HEADER_LAST_EVENT_ID, HEADER_MCP_METHOD, + HEADER_MCP_PROTOCOL_VERSION, HEADER_SESSION_ID, JSON_MIME_TYPE, }, mcp_headers, server_side_http::{ @@ -690,6 +690,99 @@ mod jsonrpc_http_status_tests { } } +#[cfg(test)] +mod standard_header_init_tests { + use super::*; + + fn initialize_message() -> ClientJsonRpcMessage { + ClientJsonRpcMessage::request( + ClientRequest::InitializeRequest(InitializeRequest { + params: InitializeRequestParams { + protocol_version: ProtocolVersion::STANDARD_HEADERS, + ..Default::default() + }, + ..Default::default() + }), + RequestId::Number(1), + ) + } + + fn headers_with(mcp_method: Option<&str>) -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert( + HEADER_MCP_PROTOCOL_VERSION, + http::HeaderValue::from_static(ProtocolVersion::STANDARD_HEADERS.as_str()), + ); + if let Some(method) = mcp_method { + headers.insert( + HEADER_MCP_METHOD, + method.parse::().unwrap(), + ); + } + headers + } + + fn no_tool_schema(_: &str) -> Option> { + None + } + + /// A supplied Mcp-Method header contradicting an initialize body must be + /// rejected at >= STANDARD_HEADERS. Regression test for + /// https://github.com/modelcontextprotocol/rust-sdk/issues/1271 + #[test] + fn initialize_rejects_contradicting_mcp_method_header() { + let headers = headers_with(Some("tools/list")); + assert!( + validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_err() + ); + } + + /// Absence of the Mcp-Method header on initialize stays accepted: clients + /// emit SEP-2243 headers only after the version has been negotiated. + #[test] + fn initialize_accepts_missing_mcp_method_header() { + let headers = headers_with(None); + assert!(validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_ok()); + } + + /// A supplied Mcp-Method header matching the initialize body is accepted. + #[test] + fn initialize_accepts_matching_mcp_method_header() { + let headers = headers_with(Some("initialize")); + assert!(validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_ok()); + } + + /// Conflicting duplicate Mcp-Method values are rejected in both orders: + /// only the first value used to be checked, so an appended contradictory + /// value passed silently. Duplicate values are rejected explicitly. + #[test] + fn initialize_rejects_conflicting_duplicates_in_both_orders() { + for (first, second) in [("initialize", "tools/list"), ("tools/list", "initialize")] { + let mut headers = headers_with(Some(first)); + headers.append( + HEADER_MCP_METHOD, + http::HeaderValue::from_str(second).unwrap(), + ); + assert!( + validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_err() + ); + } + } + + /// A present but non-UTF8 Mcp-Method value is rejected: it must not be + /// silently treated as an absent header. + #[test] + fn initialize_rejects_present_non_text_method() { + let mut headers = headers_with(None); + let value = http::HeaderValue::from_bytes(&[0xff]).unwrap(); + assert!(value.to_str().is_err()); + headers.insert(HEADER_MCP_METHOD, value); + assert!( + validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_err() + ); + } +} + fn jsonrpc_message_response( message: ServerJsonRpcMessage, map_protocol_status: bool, @@ -724,8 +817,11 @@ fn header_mismatch_jsonrpc_response( /// Validates SEP-2243 `Mcp-Method` / `Mcp-Name` / `Mcp-Param-*` headers against the body. /// /// Only enforced when the request declares a protocol version `>= STANDARD_HEADERS`. -/// The `initialize` handshake is exempt: clients emit these headers only after the -/// version has been negotiated. `tool_schema` supplies the called tool's input schema +/// The `initialize` handshake is exempt from *requiring* them: clients emit these +/// headers only after the version has been negotiated. But like +/// `validate_header_matches_init_body`, a *supplied* `Mcp-Method` header that +/// contradicts the body is rejected — middleboxes route on the header without +/// parsing the body. `tool_schema` supplies the called tool's input schema /// so annotated `Mcp-Param-*` headers can be checked (no schema => those are skipped). fn validate_standard_headers( headers: &HeaderMap, @@ -743,6 +839,35 @@ fn validate_standard_headers( let request_id = match message { ClientJsonRpcMessage::Request(req) => { if matches!(&req.request, ClientRequest::InitializeRequest(_)) { + // The handshake may omit SEP-2243 headers, but a supplied + // Mcp-Method header must still agree with the body: middleboxes + // route on the header without parsing the body. `get_all` is + // used so every supplied value is inspected: a contradictory + // duplicate must not hide behind a matching first value, and a + // present-but-non-UTF8 value is rejected rather than silently + // treated as absent. Duplicates (even identical ones) are + // rejected explicitly instead of passing by accident. + let mut method_values = headers.get_all(HEADER_MCP_METHOD).iter(); + let Some(first) = method_values.next() else { + return Ok(()); + }; + if method_values.next().is_some() { + return Err(header_mismatch_jsonrpc_response( + Some(req.id.clone()), + "multiple Mcp-Method header values supplied; initialize requires exactly one", + ) + .into()); + } + if first != http::HeaderValue::from_static("initialize") { + return Err(header_mismatch_jsonrpc_response( + Some(req.id.clone()), + format!( + "Mcp-Method header `{}` does not match body method `initialize`", + String::from_utf8_lossy(first.as_bytes()) + ), + ) + .into()); + } return Ok(()); } Some(req.id.clone()) @@ -1860,6 +1985,11 @@ where .serve_negotiated_request_directly(service, request, part) .await; } + // SEP-2243: run the standard-header validator on the initialize + // handshake before a session is created for it — a supplied + // Mcp-Method header contradicting the body must be rejected + // here, not answered with HTTP 200 and a fresh session. + validate_standard_headers(&part.headers, &message, |name| self.tool_schema(name))?; // Capture init params for external store persistence before // extensions are injected (which would require Clone). let stored_init_params = match &mut message { diff --git a/crates/rmcp/tests/test_streamable_http_standard_headers.rs b/crates/rmcp/tests/test_streamable_http_standard_headers.rs index 9e708052a..5bbfedd64 100644 --- a/crates/rmcp/tests/test_streamable_http_standard_headers.rs +++ b/crates/rmcp/tests/test_streamable_http_standard_headers.rs @@ -313,3 +313,89 @@ async fn rejects_missing_param_header_with_32020() -> anyhow::Result<()> { ct.cancel(); Ok(()) } + +/// Spawns the server in legacy session mode: the initialize handshake creates +/// a real session, exercising the path where the validator used to be bypassed. +async fn spawn_legacy_server() -> (reqwest::Client, String, CancellationToken) { + let config = StreamableHttpServerConfig::default() + .with_legacy_session_mode(true) + .with_json_response(true) + .with_sse_keep_alive(None) + .with_cancellation_token(CancellationToken::new()); + let ct = config.cancellation_token.clone(); + let service: StreamableHttpService = + StreamableHttpService::new(|| Ok(HeaderValidationServer), Default::default(), config); + + let router = axum::Router::new().nest_service("/mcp", service); + let tcp_listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = tcp_listener.local_addr().unwrap(); + tokio::spawn({ + let ct = ct.clone(); + async move { + let _ = axum::serve(tcp_listener, router) + .with_graceful_shutdown(async move { ct.cancelled_owned().await }) + .await; + } + }); + (reqwest::Client::new(), format!("http://{addr}/mcp"), ct) +} + +/// POSTs an `initialize` request with the given protocol version and optional +/// `Mcp-Method` header. +async fn post_initialize( + client: &reqwest::Client, + url: &str, + version: &str, + mcp_method: Option<&str>, +) -> reqwest::Response { + let body = serde_json::json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": version, + "capabilities": {}, + "clientInfo": { "name": "test", "version": "1.0" }, + } + }); + let mut req = client + .post(url) + .header("Content-Type", "application/json") + .header("Accept", "application/json, text/event-stream") + .header("MCP-Protocol-Version", version) + .body(body.to_string()); + if let Some(method) = mcp_method { + req = req.header("Mcp-Method", method); + } + req.send().await.expect("send initialize request") +} + +#[tokio::test] +async fn rejects_initialize_with_contradicting_mcp_method_before_session_creation() +-> anyhow::Result<()> { + let (client, url, ct) = spawn_legacy_server().await; + + // Regression test: in legacy session mode with no session id, an + // `initialize` body carrying a contradictory `Mcp-Method` header used to + // bypass the validator, returning HTTP 200 and creating a session. + let response = post_initialize(&client, &url, SEP_VERSION, Some("tools/list")).await; + assert_eq!(response.status(), 400); + let body: serde_json::Value = response.json().await?; + assert_eq!(body["error"]["code"], -32020); + + ct.cancel(); + Ok(()) +} + +#[tokio::test] +async fn accepts_initialize_with_matching_mcp_method() -> anyhow::Result<()> { + let (client, url, ct) = spawn_legacy_server().await; + + // A matching Mcp-Method header on initialize passes validation and the + // handshake completes (HTTP 200, session created). + let response = post_initialize(&client, &url, SEP_VERSION, Some("initialize")).await; + assert_eq!(response.status(), 200); + + ct.cancel(); + Ok(()) +}