mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-14 19:26:56 -04:00
Fix #1728: require the JSON-RPC 2.0 envelope and correlate control replies
The server dispatched requests and notifications without ever checking jsonrpc, so a missing or wrong version could invoke tools or flip notification state; Remote accepted any reply with ok/result/error without comparing ids, so a faulty endpoint could attribute a stray result to the current call. handle() now requires jsonrpc 2.0 before dispatch (other envelopes get -32600, or are ignored for notifications, which have no id for a reply) and Remote::call rejects a reply whose id doesn't echo the request's, dropping the connection so the next call starts fresh, with the usual uncertainty error. Three new tests cover wrong/missing versions (incl. batch entries and a state-changing notification) and the mismatch-reconnect; docs/mcp.md documents both. cargo test -p vectorcraft-mcp passes.
This commit is contained in:
1 parent
5336d80196
commit
cc518797a8
5 files changed
+102
-1
No files matched your search
@@ -94,6 +94,16 @@ impl Backend for Remote {
|
||||
format!("VectorCraft app at {}: `{method}` failed: {e}; operation may have completed; inspect state before retrying", self.addr)
|
||||
})?;
|
||||
let v: Value = serde_json::from_str(reply.trim()).map_err(|e| format!("bad reply from app: {e}"))?;
|
||||
if v.get("id") != Some(&json!(id)) {
|
||||
// Not the answer to this call: the stream is desynchronised (or the endpoint faulty),
|
||||
// so the connection is dropped and the next call starts a fresh one. The operation may
|
||||
// still have completed, as with any reply that never arrived whole (#1728).
|
||||
self.conn = None;
|
||||
return Err(format!(
|
||||
"VectorCraft app at {}: reply id mismatch (expected {id}); connection dropped; operation may have completed; inspect state before retrying",
|
||||
self.addr
|
||||
));
|
||||
}
|
||||
if v.get("ok").and_then(Value::as_bool) == Some(true) {
|
||||
Ok(v.get("result").cloned().unwrap_or(Value::Null))
|
||||
} else {
|
||||
|
||||
@@ -198,6 +198,12 @@ impl Server {
|
||||
}
|
||||
return Some(error(id.unwrap_or(Value::Null), INVALID_REQUEST, "missing `method`"));
|
||||
};
|
||||
// JSON-RPC 2.0 is the only envelope this server speaks: a request or notification in any
|
||||
// other envelope is never dispatched and never changes state (#1728).
|
||||
if o.get("jsonrpc").and_then(Value::as_str) != Some("2.0") {
|
||||
// A notification has no id to address a reply to; ignoring it is the only answer.
|
||||
return id.map(|id| error(id, INVALID_REQUEST, "expected `\"jsonrpc\": \"2.0\"`"));
|
||||
};
|
||||
let params = o.get("params").cloned().unwrap_or(Value::Null);
|
||||
let Some(id) = id else {
|
||||
self.notification(method, ¶ms);
|
||||
|
||||
@@ -506,6 +506,42 @@ fn remote_does_not_replay_when_edit_reply_is_lost() {
|
||||
assert!(next.is_ok(), "the next call must reconnect: {next:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_rejects_a_reply_with_a_mismatched_id_and_reconnects() {
|
||||
// A fake control server that answers the first request with another call's id, then serves
|
||||
// correctly: the client must reject the stray reply, drop the connection, and reconnect.
|
||||
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
let addr = listener.local_addr().unwrap().to_string();
|
||||
let h = std::thread::spawn(move || {
|
||||
let mut seen = vec![];
|
||||
for stream in listener.incoming().flatten().take(2) {
|
||||
stream.set_read_timeout(Some(std::time::Duration::from_secs(5))).unwrap();
|
||||
let mut out = stream.try_clone().unwrap();
|
||||
let mut reader = BufReader::new(stream);
|
||||
let mut line = String::new();
|
||||
if reader.read_line(&mut line).unwrap_or(0) == 0 {
|
||||
continue;
|
||||
}
|
||||
let msg: Value = serde_json::from_str(&line).unwrap();
|
||||
// First connection: somebody else's answer. Later ones: the right echo.
|
||||
let id = if seen.is_empty() { json!(msg["id"].as_u64().unwrap_or(0) + 1000) } else { msg["id"].clone() };
|
||||
let reply = json!({"ok": true, "result": {"echo": msg["params"]}, "id": id});
|
||||
writeln!(out, "{reply}").unwrap();
|
||||
seen.push(msg);
|
||||
}
|
||||
seen
|
||||
});
|
||||
let mut remote = Remote::connect(&addr).unwrap();
|
||||
let err = remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {}})).expect_err("a stray reply must not be trusted");
|
||||
assert!(err.contains("mismatch"), "{err}");
|
||||
let good = remote.call("engine.execute", json!({"command": "shape.rectangle", "params": {}}));
|
||||
assert!(good.is_ok(), "the next call reconnects: {good:?}");
|
||||
drop(remote);
|
||||
let seen = h.join().unwrap();
|
||||
assert_eq!(seen.len(), 2, "one request per connection: {seen:?}");
|
||||
assert_ne!(seen[0]["id"], seen[1]["id"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_connect_fails_fast() {
|
||||
// A port this test holds, bound but not listening, so connecting to it is refused. (A port
|
||||
|
||||
@@ -97,6 +97,50 @@ fn prompt_argument_errors_are_invalid_params() {
|
||||
assert_eq!(err_code(&mut s, 3, "prompts/get", json!({})), -32602);
|
||||
}
|
||||
|
||||
// ---------- envelope ----------
|
||||
|
||||
/// Requests and notifications outside a `"jsonrpc": "2.0"` envelope are never dispatched (#1728).
|
||||
#[test]
|
||||
fn envelopes_without_version_20_are_not_dispatched() {
|
||||
let mut s = server();
|
||||
// A request in no envelope: invalid, and nothing ran.
|
||||
let line = json!({"id": 1, "method": "ping", "params": {}}).to_string();
|
||||
let v: Value = serde_json::from_str(&s.handle_line(&line).unwrap()).unwrap();
|
||||
assert_eq!(v["error"]["code"], -32600, "{v}");
|
||||
assert!(v["error"]["message"].as_str().unwrap_or_default().contains("2.0"), "{v}");
|
||||
// A wrong version is the same.
|
||||
let line = json!({"jsonrpc": "1.0", "id": 2, "method": "ping", "params": {}}).to_string();
|
||||
let v: Value = serde_json::from_str(&s.handle_line(&line).unwrap()).unwrap();
|
||||
assert_eq!(v["error"]["code"], -32600, "{v}");
|
||||
// In a batch the unversioned message is an error entry, not a dispatch; the 2.0 one answers.
|
||||
let batch = json!([
|
||||
{"jsonrpc": "2.0", "id": 3, "method": "ping", "params": {}},
|
||||
{"id": 4, "method": "ping", "params": {}},
|
||||
])
|
||||
.to_string();
|
||||
let v: Value = serde_json::from_str(&s.handle_line(&batch).unwrap()).unwrap();
|
||||
assert_eq!(v.as_array().map(Vec::len), Some(2), "{v}");
|
||||
assert!(v[0].get("result").is_some(), "{v}");
|
||||
assert_eq!(v[1]["error"]["code"], -32600, "{v}");
|
||||
// The 2.0 request still works.
|
||||
assert_eq!(result(&mut s, 5, "ping", json!({})), json!({}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn notification_without_version_20_changes_nothing() {
|
||||
let mut s = server();
|
||||
assert!(!s.is_initialized());
|
||||
// No envelope, no id: there is no address for a reply, so it is ignored — and it must not
|
||||
// flip the server into its initialised state.
|
||||
let line = json!({"method": "notifications/initialized", "params": {}}).to_string();
|
||||
assert!(s.handle_line(&line).is_none());
|
||||
assert!(!s.is_initialized(), "an unversioned notification changed state");
|
||||
// The 2.0 notification still works.
|
||||
let line = json!({"jsonrpc": "2.0", "method": "notifications/initialized", "params": {}}).to_string();
|
||||
assert!(s.handle_line(&line).is_none());
|
||||
assert!(s.is_initialized());
|
||||
}
|
||||
|
||||
// ---------- completions ----------
|
||||
|
||||
#[test]
|
||||
|
||||
+6
-1
@@ -105,7 +105,12 @@ given rather than guarding against other software on the computer.
|
||||
## Protocol
|
||||
|
||||
Newline-delimited JSON-RPC 2.0 on stdio. The revision is **`2025-06-18`**; `2025-03-26` and `2024-11-05` are
|
||||
accepted too. Clients requesting `2026-07-28` through `initialize`, or through a request's
|
||||
accepted too. Every request and notification must carry `"jsonrpc": "2.0"`: anything else is
|
||||
refused with `-32600` (requests) or ignored (notifications, which have no id to address a reply
|
||||
to) before it is dispatched. When a tool call reaches a running app, it is one control-channel
|
||||
request, and the app echoes its `id` in the reply: a reply with any other `id` is rejected, the
|
||||
connection dropped, and the next call starts a fresh one — the call reports uncertainty like any
|
||||
reply that never arrived whole. Clients requesting `2026-07-28` through `initialize`, or through a request's
|
||||
`_meta["io.modelcontextprotocol/protocolVersion"]`, receive `resultType: "complete"`,
|
||||
`ttlMs` and `cacheScope: "private"` on tool/resource/template/prompt lists and resource/prompt reads.
|
||||
List TTL is 600000 ms; live reads use zero. Older clients keep the original response shapes.
|
||||
|
||||
Reference in new issue
Block a user