diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 5fc461f0d..9dd04564e 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -373,6 +373,9 @@ impl<'de> Deserialize<'de> for NumberOrString { } else { Err(serde::de::Error::custom("Number too large for i64")) } + } else if let Some(i) = n.as_f64().and_then(integral_f64_to_i64) { + // `1.0` and `1e2` are integers written in float form + Ok(NumberOrString::Number(i)) } else { Err(serde::de::Error::custom("Expected an integer")) } @@ -383,6 +386,14 @@ impl<'de> Deserialize<'de> for NumberOrString { } } +/// The integer a JSON number in float form stands for, when it is one that an +/// `f64` represents exactly. 2^53 itself is excluded: 2^53 + 1 parses to the +/// same `f64`, so that value cannot say which integer was sent. +fn integral_f64_to_i64(f: f64) -> Option { + const MAX_EXACT: f64 = 9_007_199_254_740_992.0; // 2^53 + (f.fract() == 0.0 && f.abs() < MAX_EXACT).then_some(f as i64) +} + #[cfg(feature = "schemars")] impl schemars::JsonSchema for NumberOrString { fn schema_name() -> Cow<'static, str> { diff --git a/crates/rmcp/src/transport/async_rw.rs b/crates/rmcp/src/transport/async_rw.rs index 59d5eccdd..af9697361 100644 --- a/crates/rmcp/src/transport/async_rw.rs +++ b/crates/rmcp/src/transport/async_rw.rs @@ -902,4 +902,49 @@ mod test { }), ); } + + #[cfg(feature = "server")] + #[tokio::test] + async fn receive_rejects_request_with_unusable_id() { + use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; + + use crate::{RoleServer, transport::Transport}; + + let (server_io, client_io) = tokio::io::duplex(4096); + let (server_r, server_w) = tokio::io::split(server_io); + let (client_r, mut client_w) = tokio::io::split(client_io); + + let mut transport = AsyncRwTransport::::new(server_r, server_w); + + // A request whose `id` is not an integer or a string, followed by a valid + // notification. The request must be answered with an error, not read as a + // notification and dropped, or the peer waits for a response forever. + client_w + .write_all( + b"{\"jsonrpc\":\"2.0\",\"id\":1.5,\"method\":\"tools/list\"}\n{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n", + ) + .await + .unwrap(); + + let received = transport + .receive() + .await + .expect("transport should reject the request and yield the next valid message"); + assert_eq!( + serde_json::to_value(&received).unwrap()["method"], + "notifications/initialized", + ); + + let mut reply_buf = Vec::new(); + let mut peer = BufReader::new(client_r); + peer.read_until(b'\n', &mut reply_buf).await.unwrap(); + let reply: serde_json::Value = serde_json::from_slice(&reply_buf).unwrap(); + assert_eq!( + reply, + serde_json::json!({ + "jsonrpc": "2.0", + "error": {"code": -32600, "message": "Invalid request"}, + }), + ); + } } diff --git a/crates/rmcp/tests/test_deserialization.rs b/crates/rmcp/tests/test_deserialization.rs index bac8e180b..1f2191864 100644 --- a/crates/rmcp/tests/test_deserialization.rs +++ b/crates/rmcp/tests/test_deserialization.rs @@ -397,3 +397,62 @@ mod arbitrary_precision { assert_eq!(amount.default, Some(1.5)); } } + +/// A message that carries `id` and `method` is a request. When the `id` is not +/// one the SDK can represent, the message is invalid; it must not be read as a +/// notification, which would leave the peer waiting for a response. +mod request_id { + use rmcp::model::{ClientJsonRpcMessage, JsonRpcMessage, NumberOrString}; + + fn parse(id: &str, method: &str) -> Result { + serde_json::from_str(&format!( + r#"{{"jsonrpc":"2.0","id":{id},"method":"{method}"}}"# + )) + } + + #[test] + fn request_with_unusable_id_is_rejected() { + for id in [ + "9223372036854775808", + "1.5", + "9007199254740993.0", + "true", + "[1]", + "{}", + ] { + for method in ["ping", "tools/list", "notifications/initialized"] { + let result = parse(id, method); + assert!( + result.is_err(), + "id {id} with method {method} parsed as {result:?}" + ); + } + } + } + + #[test] + fn integral_id_in_float_form_is_that_integer() { + for (id, expected) in [("1.0", 1), ("1e2", 100), ("-3.0", -3)] { + let JsonRpcMessage::Request(request) = parse(id, "ping").unwrap() else { + panic!("id {id} did not parse as a request"); + }; + assert_eq!(request.id, NumberOrString::Number(expected), "id {id}"); + } + } + + #[test] + fn largest_i64_id_is_kept() { + let JsonRpcMessage::Request(request) = parse("9223372036854775807", "ping").unwrap() else { + panic!("expected a request"); + }; + assert_eq!(request.id, NumberOrString::Number(i64::MAX)); + } + + #[test] + fn notification_still_parses() { + let message: ClientJsonRpcMessage = + serde_json::from_str(r#"{"jsonrpc":"2.0","method":"notifications/initialized"}"#) + .unwrap(); + assert!(matches!(message, JsonRpcMessage::Notification(_))); + } +}