Skip to content

Commit 017e1c3

Browse files
talhaymeclaude
andauthored
fix(model): read a request id written in float form as that integer (#1346)
* fix(model): read a request id written in float form as that integer A JSON-RPC id of 1.0 or 1e2 is an integer written the way some encoders write every number. It used to be rejected, so the request got no response. NumberOrString now accepts a float that an f64 represents exactly as an integer (up to 2^53) and reads it as that integer. #1287 already makes a message with an id and a method parse only as a request, so the earlier change to JsonRpcNotification is dropped; the tests for an unusable id stay as regression tests. Refs #1345 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(model): reject 2^53 as a float-form id; it cannot say which integer was sent 2^53 + 1 written in float form parses to the same f64 as 2^53, so accepting 2^53 silently turned 9007199254740993.0 into 9007199254740992. The exact-integer range is now strictly below 2^53. Added that id to request_with_unusable_id_is_rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ad5a284 commit 017e1c3

3 files changed

Lines changed: 115 additions & 0 deletions

File tree

‎crates/rmcp/src/model.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,9 @@ impl<'de> Deserialize<'de> for NumberOrString {
373373
} else {
374374
Err(serde::de::Error::custom("Number too large for i64"))
375375
}
376+
} else if let Some(i) = n.as_f64().and_then(integral_f64_to_i64) {
377+
// `1.0` and `1e2` are integers written in float form
378+
Ok(NumberOrString::Number(i))
376379
} else {
377380
Err(serde::de::Error::custom("Expected an integer"))
378381
}
@@ -383,6 +386,14 @@ impl<'de> Deserialize<'de> for NumberOrString {
383386
}
384387
}
385388

389+
/// The integer a JSON number in float form stands for, when it is one that an
390+
/// `f64` represents exactly. 2^53 itself is excluded: 2^53 + 1 parses to the
391+
/// same `f64`, so that value cannot say which integer was sent.
392+
fn integral_f64_to_i64(f: f64) -> Option<i64> {
393+
const MAX_EXACT: f64 = 9_007_199_254_740_992.0; // 2^53
394+
(f.fract() == 0.0 && f.abs() < MAX_EXACT).then_some(f as i64)
395+
}
396+
386397
#[cfg(feature = "schemars")]
387398
impl schemars::JsonSchema for NumberOrString {
388399
fn schema_name() -> Cow<'static, str> {

‎crates/rmcp/src/transport/async_rw.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -902,4 +902,49 @@ mod test {
902902
}),
903903
);
904904
}
905+
906+
#[cfg(feature = "server")]
907+
#[tokio::test]
908+
async fn receive_rejects_request_with_unusable_id() {
909+
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
910+
911+
use crate::{RoleServer, transport::Transport};
912+
913+
let (server_io, client_io) = tokio::io::duplex(4096);
914+
let (server_r, server_w) = tokio::io::split(server_io);
915+
let (client_r, mut client_w) = tokio::io::split(client_io);
916+
917+
let mut transport = AsyncRwTransport::<RoleServer, _, _>::new(server_r, server_w);
918+
919+
// A request whose `id` is not an integer or a string, followed by a valid
920+
// notification. The request must be answered with an error, not read as a
921+
// notification and dropped, or the peer waits for a response forever.
922+
client_w
923+
.write_all(
924+
b"{\"jsonrpc\":\"2.0\",\"id\":1.5,\"method\":\"tools/list\"}\n{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n",
925+
)
926+
.await
927+
.unwrap();
928+
929+
let received = transport
930+
.receive()
931+
.await
932+
.expect("transport should reject the request and yield the next valid message");
933+
assert_eq!(
934+
serde_json::to_value(&received).unwrap()["method"],
935+
"notifications/initialized",
936+
);
937+
938+
let mut reply_buf = Vec::new();
939+
let mut peer = BufReader::new(client_r);
940+
peer.read_until(b'\n', &mut reply_buf).await.unwrap();
941+
let reply: serde_json::Value = serde_json::from_slice(&reply_buf).unwrap();
942+
assert_eq!(
943+
reply,
944+
serde_json::json!({
945+
"jsonrpc": "2.0",
946+
"error": {"code": -32600, "message": "Invalid request"},
947+
}),
948+
);
949+
}
905950
}

‎crates/rmcp/tests/test_deserialization.rs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,3 +397,62 @@ mod arbitrary_precision {
397397
assert_eq!(amount.default, Some(1.5));
398398
}
399399
}
400+
401+
/// A message that carries `id` and `method` is a request. When the `id` is not
402+
/// one the SDK can represent, the message is invalid; it must not be read as a
403+
/// notification, which would leave the peer waiting for a response.
404+
mod request_id {
405+
use rmcp::model::{ClientJsonRpcMessage, JsonRpcMessage, NumberOrString};
406+
407+
fn parse(id: &str, method: &str) -> Result<ClientJsonRpcMessage, serde_json::Error> {
408+
serde_json::from_str(&format!(
409+
r#"{{"jsonrpc":"2.0","id":{id},"method":"{method}"}}"#
410+
))
411+
}
412+
413+
#[test]
414+
fn request_with_unusable_id_is_rejected() {
415+
for id in [
416+
"9223372036854775808",
417+
"1.5",
418+
"9007199254740993.0",
419+
"true",
420+
"[1]",
421+
"{}",
422+
] {
423+
for method in ["ping", "tools/list", "notifications/initialized"] {
424+
let result = parse(id, method);
425+
assert!(
426+
result.is_err(),
427+
"id {id} with method {method} parsed as {result:?}"
428+
);
429+
}
430+
}
431+
}
432+
433+
#[test]
434+
fn integral_id_in_float_form_is_that_integer() {
435+
for (id, expected) in [("1.0", 1), ("1e2", 100), ("-3.0", -3)] {
436+
let JsonRpcMessage::Request(request) = parse(id, "ping").unwrap() else {
437+
panic!("id {id} did not parse as a request");
438+
};
439+
assert_eq!(request.id, NumberOrString::Number(expected), "id {id}");
440+
}
441+
}
442+
443+
#[test]
444+
fn largest_i64_id_is_kept() {
445+
let JsonRpcMessage::Request(request) = parse("9223372036854775807", "ping").unwrap() else {
446+
panic!("expected a request");
447+
};
448+
assert_eq!(request.id, NumberOrString::Number(i64::MAX));
449+
}
450+
451+
#[test]
452+
fn notification_still_parses() {
453+
let message: ClientJsonRpcMessage =
454+
serde_json::from_str(r#"{"jsonrpc":"2.0","method":"notifications/initialized"}"#)
455+
.unwrap();
456+
assert!(matches!(message, JsonRpcMessage::Notification(_)));
457+
}
458+
}

0 commit comments

Comments
 (0)