fix: unify JSON-RPC message validation policy across transports#212
Merged
Conversation
The WebSocket and SSE receive paths validated messages with the strict isJsonRpcMessage guard and silently dropped anything that failed it, so a lenient peer's response (e.g. error:null alongside result) or request (e.g. missing the jsonrpc field) never settled the caller's promise — the same shapes worked over stdio, which only skips non-object lines. Relax the per-transport guards (ws-stream, ws-server, sse, HTTP server POST) to an object check and let the connection layer own the accept/reject policy: it dispatches lenient requests, fast-rejects structurally invalid responses with RequestError.invalidRequest, and logs anything else, as of #210. Non-object payloads are still rejected at the transport with a warning (or HTTP 400) since they carry no id to answer. Also consolidates ws-stream's private isRecord duplicate. Fixes #211 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #211
Problem
Message validation differed by transport. The stdio path (
ndJsonStream) skips only non-object JSON lines and lets the connection layer handle everything object-shaped — which it does robustly since #210 (dispatches lenient requests, fast-rejects structurally invalid responses withRequestError.invalidRequest). The WebSocket and SSE paths instead validated with the strictisJsonRpcMessageguard and silently dropped anything that failed it:{"jsonrpc":"2.0","id":1,"result":{...},"error":null}) was dropped, so the caller'ssendRequestpromise hung until connection close — there is no request timeout.jsonrpcfield got neither processing nor the spec-mandated-32600error response.The same messages worked over stdio.
Fix
Relax the per-transport guards to the shared
isRecordobject check and letConnectionown the accept/reject policy in one place:ws-stream.ts(client) andsse.ts: non-object payloads are skipped with a warning; object-shaped messages are forwarded to the connection layer. Also removesws-stream.ts's private duplicate ofisRecord.ws-server.ts: same relaxation post-initialization; the pre-initialization handshake still strictly requires a validinitializerequest, and the batch-array rejection is unchanged.server.ts(HTTP POST): non-object bodies still get 400; object-shaped messages — including client responses to server-initiated requests, which previously 400'd and hung the server-side request — are forwarded.Non-object payloads remain a transport-level rejection everywhere because they carry no
idto answer and would previously have thrown on theinoperator (hardened in #210'sreceiveMessageguard as a second line of defense).Tests
Updated the WS stream filtering test to assert lenient objects pass through (and clarified that byte frames were always ignored by the browser-style fake socket — the old single-read assertion couldn't distinguish), and added SSE pass-through and non-object-skip tests. Full
npm run checkpasses (634 tests).🤖 Generated with Claude Code