From db7a512e6a4f3a73c58b2368d1f8cfcb33c3a7bc Mon Sep 17 00:00:00 2001 From: Greg Bougeard Date: Thu, 13 Aug 2026 19:16:19 +0200 Subject: [PATCH] fix(transport): propagate real clientInfo to peer_info in stateless mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit peer_info_for_stateless_request always attached Implementation::default() (the rmcp crate's own name/version) to the synthesized Peer for every stateless streamable-HTTP request, discarding the real client_info the client sent in its initialize body. This is what serve_inner logs as "Service initialized as server"/"as client", so every real client showed up identically as "rmcp"/"" in server logs, and any handler reading context.peer.peer_info() saw the same placeholder instead of the real identity — even though the initialize handler's own typed argument correctly carried the real value all along. For the initialize request specifically, the full InitializeRequestParams (protocol version, capabilities, and client_info) is already available in the request body, so use it verbatim instead of only extracting the protocol version. Other requests still fall back to the placeholder, since stateless mode has no session to recover the original handshake from. Adds test_stateless_client_info.rs, which fails against the previous behavior and passes with this fix. --- crates/rmcp/Cargo.toml | 5 + .../transport/streamable_http_server/tower.rs | 36 +++-- .../rmcp/tests/test_stateless_client_info.rs | 145 ++++++++++++++++++ 3 files changed, 172 insertions(+), 14 deletions(-) create mode 100644 crates/rmcp/tests/test_stateless_client_info.rs diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index 9dd27a251..da4991e62 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -305,6 +305,11 @@ name = "test_stateless_protocol_version" required-features = ["server", "transport-streamable-http-server", "reqwest"] path = "tests/test_stateless_protocol_version.rs" +[[test]] +name = "test_stateless_client_info" +required-features = ["server", "transport-streamable-http-server", "reqwest"] +path = "tests/test_stateless_client_info.rs" + [[test]] name = "test_protocol_version_negotiation" required-features = ["server", "client"] diff --git a/crates/rmcp/src/transport/streamable_http_server/tower.rs b/crates/rmcp/src/transport/streamable_http_server/tower.rs index f1fef585f..b2656e8d3 100644 --- a/crates/rmcp/src/transport/streamable_http_server/tower.rs +++ b/crates/rmcp/src/transport/streamable_http_server/tower.rs @@ -2076,27 +2076,35 @@ where } /// Build a `ClientInfo` (peer_info) for a stateless request so that - /// `context.protocol_version()` returns the correct value inside handlers. + /// `context.protocol_version()` returns the correct value inside handlers, + /// and so that `Peer::peer_info()` (and therefore the `serve_inner` + /// "Service initialized as server"/"as client" log) reflects the real + /// client identity for a genuine handshake, instead of a placeholder. /// /// `serve_directly` skips the MCP handshake and accepts `peer_info = None`, /// which means `context.protocol_version()` is always `None` in stateless mode. - /// We reconstruct the protocol version from the available signal per request type: - /// - initialize: version is in the request body params (authoritative) - /// - all other requests: version is in the MCP-Protocol-Version header - /// (validated before this point; absent header defaults to 2025-03-26) + /// We reconstruct the peer info from the available signal per request type: + /// - initialize: the full `InitializeRequestParams` — protocol version, + /// capabilities, and client_info — is right there in the request body + /// (authoritative), so it is used verbatim. + /// - all other requests: there is no session to recover the original + /// handshake from, so only the protocol version can be reconstructed, + /// from the MCP-Protocol-Version header (validated before this point; + /// absent header defaults to 2025-03-26). `capabilities`/`client_info` + /// remain placeholders in this case, since stateless mode has nowhere + /// to persist the real values between requests. fn peer_info_for_stateless_request( request: &crate::model::JsonRpcRequest, headers: &HeaderMap, ) -> Option { - let version = if let ClientRequest::InitializeRequest(ref init) = request.request { - init.params.protocol_version.clone() - } else { - headers - .get(HEADER_MCP_PROTOCOL_VERSION) - .and_then(|v| v.to_str().ok()) - .and_then(|s| serde_json::from_value(serde_json::Value::String(s.to_owned())).ok()) - .unwrap_or(ProtocolVersion::V_2025_03_26) - }; + if let ClientRequest::InitializeRequest(ref init) = request.request { + return Some(init.params.clone()); + } + let version = headers + .get(HEADER_MCP_PROTOCOL_VERSION) + .and_then(|v| v.to_str().ok()) + .and_then(|s| serde_json::from_value(serde_json::Value::String(s.to_owned())).ok()) + .unwrap_or(ProtocolVersion::V_2025_03_26); Some(InitializeRequestParams { meta: None, protocol_version: version, diff --git a/crates/rmcp/tests/test_stateless_client_info.rs b/crates/rmcp/tests/test_stateless_client_info.rs new file mode 100644 index 000000000..903ae2136 --- /dev/null +++ b/crates/rmcp/tests/test_stateless_client_info.rs @@ -0,0 +1,145 @@ +//! Regression test for stateless streamable HTTP discarding the real +//! `clientInfo` from a genuine `initialize` request. +//! +//! Before the fix, `peer_info_for_stateless_request` always attached +//! `Implementation::default()` (i.e. `{"name": "rmcp", "version": ""}`) to the synthesized `Peer`, regardless of what the client +//! actually sent in its `initialize` body. That value is what +//! `serve_inner` logs as "Service initialized as server"/"as client", and +//! what any handler sees via `context.peer.peer_info()` — so every real +//! client (regardless of its own identity) appeared identical, and +//! indistinguishable from rmcp's own placeholder, in stateless mode. +#![cfg(not(feature = "local"))] + +use rmcp::{ + ErrorData, RoleServer, ServerHandler, + model::{ + Implementation, InitializeRequestParams, InitializeResult, ServerCapabilities, ServerInfo, + }, + service::RequestContext, + transport::streamable_http_server::{ + StreamableHttpServerConfig, StreamableHttpService, session::local::LocalSessionManager, + }, +}; +use tokio_util::sync::CancellationToken; + +/// Echoes both the `clientInfo` it received as a typed handler argument and +/// the `clientInfo` visible via `context.peer.peer_info()` at that same +/// instant, via `InitializeResult::instructions`, so a black-box HTTP test +/// can compare the two without any shared in-process state. +#[derive(Clone, Default)] +struct EchoingPeerInfo; + +impl ServerHandler for EchoingPeerInfo { + fn get_info(&self) -> ServerInfo { + ServerInfo::new(ServerCapabilities::default()) + } + + async fn initialize( + &self, + request: InitializeRequestParams, + context: RequestContext, + ) -> Result { + let peer_client_info: Option = + context.peer.peer_info().map(|p| p.client_info.clone()); + let mut info = self.get_info(); + info.instructions = Some( + serde_json::json!({ + "request_client_info": request.client_info, + "peer_client_info": peer_client_info, + }) + .to_string(), + ); + Ok(info) + } +} + +fn stateless_json_config() -> StreamableHttpServerConfig { + StreamableHttpServerConfig::default() + .with_legacy_session_mode(false) + .with_sse_keep_alive(None) + .with_cancellation_token(CancellationToken::new()) + .with_json_response(true) +} + +async fn spawn_server_of( + config: StreamableHttpServerConfig, +) -> (reqwest::Client, String, CancellationToken) { + let ct = config.cancellation_token.clone(); + let service: StreamableHttpService = + StreamableHttpService::new(|| Ok(H::default()), 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) +} + +async fn post_init_with_client_info( + client: &reqwest::Client, + url: &str, + client_name: &str, + client_version: &str, +) -> serde_json::Value { + let body = serde_json::json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": {"name": client_name, "version": client_version} + } + }); + let resp = client + .post(url) + .header("Content-Type", "application/json") + .header("Accept", "application/json, text/event-stream") + .body(body.to_string()) + .send() + .await + .expect("send request"); + assert!(resp.status().is_success(), "HTTP {}", resp.status()); + resp.json().await.expect("parse JSON") +} + +#[tokio::test] +async fn stateless_init_propagates_real_client_info_to_peer_info() { + let (client, url, ct) = spawn_server_of::(stateless_json_config()).await; + + let resp = post_init_with_client_info(&client, &url, "totally-real-client", "42.0.0").await; + + let instructions: serde_json::Value = + serde_json::from_str(resp["result"]["instructions"].as_str().unwrap()) + .expect("instructions should contain the captured JSON"); + + // Sanity check: the handler's own typed argument really did carry the + // client's real identity — the request body was parsed correctly. + assert_eq!( + instructions["request_client_info"]["name"], "totally-real-client", + "sanity check failed — the handler never saw the real clientInfo" + ); + + // The fix: `context.peer.peer_info()` — the exact value `serve_inner` + // logs as "Service initialized as server" — must reflect the real + // client identity for a genuine `initialize` request, not + // `Implementation::default()`. + assert_eq!( + instructions["peer_client_info"]["name"], "totally-real-client", + "Peer::peer_info() should carry the real client_info from the \ + initialize request, not a placeholder identity" + ); + assert_eq!(instructions["peer_client_info"]["version"], "42.0.0"); + + ct.cancel(); +}