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(); +}