Skip to content

Commit 57f9063

Browse files
dsp-antclaude
andcommitted
fix: change JSON-RPC request ID type from u32 to i64
The JSON-RPC 2.0 specification allows the ID field to be any JSON number, including negative integers and large values. The previous u32 implementation was limited to 0-4,294,967,295 and couldn't handle negative IDs. Changes: - Changed NumberOrString::Number from u32 to i64 to support full JSON number range - Updated deserializer to handle both signed and unsigned integers - Modified AtomicU32Provider to use AtomicU64 internally with i64 conversion - Fixed progress token handling in meta.rs for i64 values - Added comprehensive test for negative and large request IDs This ensures full compliance with the JSON-RPC 2.0 specification. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent b514bc4 commit 57f9063

3 files changed

Lines changed: 114 additions & 14 deletions

File tree

crates/rmcp/src/model.rs

Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ impl<'de> Deserialize<'de> for ProtocolVersion {
185185
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
186186
pub enum NumberOrString {
187187
/// A numeric identifier
188-
Number(u32),
188+
Number(i64),
189189
/// A string identifier
190190
String(Arc<str>),
191191
}
@@ -227,10 +227,20 @@ impl<'de> Deserialize<'de> for NumberOrString {
227227
{
228228
let value: Value = Deserialize::deserialize(deserializer)?;
229229
match value {
230-
Value::Number(n) => Ok(NumberOrString::Number(
231-
n.as_u64()
232-
.ok_or(serde::de::Error::custom("Expect an integer"))? as u32,
233-
)),
230+
Value::Number(n) => {
231+
if let Some(i) = n.as_i64() {
232+
Ok(NumberOrString::Number(i))
233+
} else if let Some(u) = n.as_u64() {
234+
// Handle large unsigned numbers that fit in i64
235+
if u <= i64::MAX as u64 {
236+
Ok(NumberOrString::Number(u as i64))
237+
} else {
238+
Err(serde::de::Error::custom("Number too large for i64"))
239+
}
240+
} else {
241+
Err(serde::de::Error::custom("Expected an integer"))
242+
}
243+
}
234244
Value::String(s) => Ok(NumberOrString::String(s.into())),
235245
_ => Err(serde::de::Error::custom("Expect number or string")),
236246
}
@@ -1736,6 +1746,85 @@ mod tests {
17361746
assert_eq!(server_response_json, raw_response_json);
17371747
}
17381748

1749+
#[test]
1750+
fn test_negative_and_large_request_ids() {
1751+
// Test negative ID
1752+
let negative_id_json = json!({
1753+
"jsonrpc": "2.0",
1754+
"id": -1,
1755+
"method": "test",
1756+
"params": {}
1757+
});
1758+
1759+
let message: JsonRpcMessage =
1760+
serde_json::from_value(negative_id_json.clone()).expect("Should parse negative ID");
1761+
1762+
match &message {
1763+
JsonRpcMessage::Request(r) => {
1764+
assert_eq!(r.id, RequestId::Number(-1));
1765+
}
1766+
_ => panic!("Expected Request"),
1767+
}
1768+
1769+
// Test roundtrip serialization
1770+
let serialized = serde_json::to_value(&message).expect("Should serialize");
1771+
assert_eq!(serialized, negative_id_json);
1772+
1773+
// Test large negative ID
1774+
let large_negative_json = json!({
1775+
"jsonrpc": "2.0",
1776+
"id": -9007199254740991i64, // JavaScript's MIN_SAFE_INTEGER
1777+
"method": "test",
1778+
"params": {}
1779+
});
1780+
1781+
let message: JsonRpcMessage = serde_json::from_value(large_negative_json.clone())
1782+
.expect("Should parse large negative ID");
1783+
1784+
match &message {
1785+
JsonRpcMessage::Request(r) => {
1786+
assert_eq!(r.id, RequestId::Number(-9007199254740991i64));
1787+
}
1788+
_ => panic!("Expected Request"),
1789+
}
1790+
1791+
// Test large positive ID (JavaScript's MAX_SAFE_INTEGER)
1792+
let large_positive_json = json!({
1793+
"jsonrpc": "2.0",
1794+
"id": 9007199254740991i64,
1795+
"method": "test",
1796+
"params": {}
1797+
});
1798+
1799+
let message: JsonRpcMessage = serde_json::from_value(large_positive_json.clone())
1800+
.expect("Should parse large positive ID");
1801+
1802+
match &message {
1803+
JsonRpcMessage::Request(r) => {
1804+
assert_eq!(r.id, RequestId::Number(9007199254740991i64));
1805+
}
1806+
_ => panic!("Expected Request"),
1807+
}
1808+
1809+
// Test zero ID
1810+
let zero_id_json = json!({
1811+
"jsonrpc": "2.0",
1812+
"id": 0,
1813+
"method": "test",
1814+
"params": {}
1815+
});
1816+
1817+
let message: JsonRpcMessage =
1818+
serde_json::from_value(zero_id_json.clone()).expect("Should parse zero ID");
1819+
1820+
match &message {
1821+
JsonRpcMessage::Request(r) => {
1822+
assert_eq!(r.id, RequestId::Number(0));
1823+
}
1824+
_ => panic!("Expected Request"),
1825+
}
1826+
}
1827+
17391828
#[test]
17401829
fn test_protocol_version_order() {
17411830
let v1 = ProtocolVersion::V_2024_11_05;

crates/rmcp/src/model/meta.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,19 @@ impl Meta {
116116
pub fn get_progress_token(&self) -> Option<ProgressToken> {
117117
self.0.get(PROGRESS_TOKEN_FIELD).and_then(|v| match v {
118118
Value::String(s) => Some(ProgressToken(NumberOrString::String(s.to_string().into()))),
119-
Value::Number(n) => n
120-
.as_u64()
121-
.map(|n| ProgressToken(NumberOrString::Number(n as u32))),
119+
Value::Number(n) => {
120+
if let Some(i) = n.as_i64() {
121+
Some(ProgressToken(NumberOrString::Number(i)))
122+
} else if let Some(u) = n.as_u64() {
123+
if u <= i64::MAX as u64 {
124+
Some(ProgressToken(NumberOrString::Number(u as i64)))
125+
} else {
126+
None
127+
}
128+
} else {
129+
None
130+
}
131+
}
122132
_ => None,
123133
})
124134
}

crates/rmcp/src/service.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ impl<R: ServiceRole, S: Service<R>> DynService<R> for S {
193193
use std::{
194194
collections::{HashMap, VecDeque},
195195
ops::Deref,
196-
sync::{Arc, atomic::AtomicU32},
196+
sync::{Arc, atomic::AtomicU64},
197197
time::Duration,
198198
};
199199

@@ -212,20 +212,21 @@ pub type AtomicU32ProgressTokenProvider = AtomicU32Provider;
212212

213213
#[derive(Debug, Default)]
214214
pub struct AtomicU32Provider {
215-
id: AtomicU32,
215+
id: AtomicU64,
216216
}
217217

218218
impl RequestIdProvider for AtomicU32Provider {
219219
fn next_request_id(&self) -> RequestId {
220-
RequestId::Number(self.id.fetch_add(1, std::sync::atomic::Ordering::SeqCst))
220+
let id = self.id.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
221+
// Safe conversion: we start at 0 and increment by 1, so we won't overflow i64::MAX in practice
222+
RequestId::Number(id as i64)
221223
}
222224
}
223225

224226
impl ProgressTokenProvider for AtomicU32Provider {
225227
fn next_progress_token(&self) -> ProgressToken {
226-
ProgressToken(NumberOrString::Number(
227-
self.id.fetch_add(1, std::sync::atomic::Ordering::SeqCst),
228-
))
228+
let id = self.id.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
229+
ProgressToken(NumberOrString::Number(id as i64))
229230
}
230231
}
231232

0 commit comments

Comments
 (0)