Skip to content

Commit 64d22de

Browse files
authored
fix: don't respond to unparseable messages (#940)
* fix: don't respond to unparseable messages * docs: spell 'unparsable' to satisfy typos linter * fix: only ignore unparsable JSON, keep protocol errors visible Classify the serde error in the receive loop: syntax/EOF errors are unparsable input with no correlatable id (issue #938) and stay silent, while data errors (valid JSON that doesn't match the message shape) are real protocol errors and get an error response instead of being dropped. Add a test covering the protocol-error path. * fix: respond with Invalid Request for malformed protocol messages --------- Co-authored-by: tsouth89 <tsouth89@users.noreply.github.com>
1 parent 288f996 commit 64d22de

1 file changed

Lines changed: 79 additions & 22 deletions

File tree

crates/rmcp/src/transport/async_rw.rs

Lines changed: 79 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,30 @@ where
143143
Ok(Some(msg)) => return Some(msg),
144144
Ok(None) => continue,
145145
Err(JsonRpcMessageCodecError::Serde(e)) => {
146-
tracing::debug!("Parse error on incoming message: {e}");
147-
let mut write = self.write.lock().await;
148-
let framed = write.as_mut()?;
149-
let response = TxJsonRpcMessage::<Role>::error(
150-
ErrorData::parse_error("Parse error", None),
151-
None,
152-
);
153-
if framed.send(response).await.is_err() {
154-
return None;
146+
match e.classify() {
147+
serde_json::error::Category::Syntax | serde_json::error::Category::Eof => {
148+
// The input isn't valid JSON, so there's no message id to correlate a
149+
// response to, and replying to invalid data can trigger an error storm
150+
// if the peer echoes the response back as more invalid data. This
151+
// matches the other official MCP SDKs, which ignore unparsable input.
152+
// See https://github.com/modelcontextprotocol/rust-sdk/issues/938
153+
tracing::debug!("Ignoring unparsable incoming message: {e}");
154+
}
155+
serde_json::error::Category::Data | serde_json::error::Category::Io => {
156+
// Well-formed JSON that doesn't match the expected message shape is a
157+
// real protocol error rather than unparsable input, so surface it with
158+
// an Invalid Request response instead of silently dropping it.
159+
tracing::debug!("Protocol error on incoming message: {e}");
160+
let mut write = self.write.lock().await;
161+
let framed = write.as_mut()?;
162+
let response = TxJsonRpcMessage::<Role>::error(
163+
ErrorData::invalid_request("Invalid request", None),
164+
None,
165+
);
166+
if framed.send(response).await.is_err() {
167+
return None;
168+
}
169+
}
155170
}
156171
}
157172
Err(e) => {
@@ -618,8 +633,8 @@ mod test {
618633

619634
#[cfg(feature = "server")]
620635
#[tokio::test]
621-
async fn receive_recovers_from_parse_error() {
622-
use tokio::io::AsyncWriteExt;
636+
async fn receive_ignores_parse_error() {
637+
use tokio::io::{AsyncReadExt, AsyncWriteExt};
623638

624639
use crate::{RoleServer, transport::Transport};
625640

@@ -638,28 +653,70 @@ mod test {
638653
.await
639654
.unwrap();
640655

656+
// The unparsable line is skipped and the next valid message is still yielded.
641657
let received = transport
642658
.receive()
643659
.await
644-
.expect("transport should recover and yield the next valid message");
660+
.expect("transport should skip the invalid line and yield the next valid message");
661+
assert_eq!(
662+
serde_json::to_value(&received).unwrap()["method"],
663+
"notifications/initialized",
664+
);
665+
666+
// No response is sent back for the unparsable message (issue #938). Dropping the
667+
// transport closes its write side, so the peer reads to EOF and should see no bytes.
668+
drop(transport);
669+
let mut reply_buf = Vec::new();
670+
client_r.read_to_end(&mut reply_buf).await.unwrap();
671+
assert!(
672+
reply_buf.is_empty(),
673+
"expected no response to an unparsable message, got: {}",
674+
String::from_utf8_lossy(&reply_buf),
675+
);
676+
}
677+
678+
#[cfg(feature = "server")]
679+
#[tokio::test]
680+
async fn receive_responds_to_protocol_error() {
681+
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
682+
683+
use crate::{RoleServer, transport::Transport};
684+
685+
let (server_io, client_io) = tokio::io::duplex(4096);
686+
let (server_r, server_w) = tokio::io::split(server_io);
687+
let (client_r, mut client_w) = tokio::io::split(client_io);
688+
689+
let mut transport = AsyncRwTransport::<RoleServer, _, _>::new(server_r, server_w);
690+
691+
// Well-formed JSON that does not match the JSON-RPC message shape, followed by a
692+
// valid notification. Unlike unparsable bytes, this is a protocol error: the
693+
// transport should reply to it and still yield the next valid message.
694+
client_w
695+
.write_all(
696+
b"{\"foo\":\"bar\"}\n{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n",
697+
)
698+
.await
699+
.unwrap();
645700

646-
// Read one line back from the peer side and parse as JSON.
701+
let received = transport.receive().await.expect(
702+
"transport should reply to the protocol error and yield the next valid message",
703+
);
704+
assert_eq!(
705+
serde_json::to_value(&received).unwrap()["method"],
706+
"notifications/initialized",
707+
);
708+
709+
// A protocol error gets an error response back (id omitted since it can't be read).
647710
let mut reply_buf = Vec::new();
648-
let mut peer = tokio::io::BufReader::new(&mut client_r);
711+
let mut peer = BufReader::new(client_r);
649712
peer.read_until(b'\n', &mut reply_buf).await.unwrap();
650713
let reply: serde_json::Value = serde_json::from_slice(&reply_buf).unwrap();
651-
652-
// Per MCP 2025-11-25: id is omitted when the server can't read the request id.
653714
assert_eq!(
654715
reply,
655716
serde_json::json!({
656717
"jsonrpc": "2.0",
657-
"error": {"code": -32700, "message": "Parse error"},
658-
})
659-
);
660-
assert_eq!(
661-
serde_json::to_value(&received).unwrap()["method"],
662-
"notifications/initialized",
718+
"error": {"code": -32600, "message": "Invalid request"},
719+
}),
663720
);
664721
}
665722
}

0 commit comments

Comments
 (0)