From 4cee2067aecc4e99ed09e7635e0294e910eebd9d Mon Sep 17 00:00:00 2001 From: utkarshsharma19 Date: Mon, 13 Jul 2026 15:31:50 -0500 Subject: [PATCH] fix(server): reply to invalid stdio JSON-RPC frames --- packages/core-internal/src/shared/stdio.ts | 17 ++++- .../core-internal/test/shared/stdio.test.ts | 17 ++++- packages/server/src/server/stdio.ts | 62 ++++++++++++++++-- packages/server/test/server/stdio.test.ts | 64 +++++++++++++++++-- 4 files changed, 146 insertions(+), 14 deletions(-) diff --git a/packages/core-internal/src/shared/stdio.ts b/packages/core-internal/src/shared/stdio.ts index 8bd794b87b..77b8c3dfce 100644 --- a/packages/core-internal/src/shared/stdio.ts +++ b/packages/core-internal/src/shared/stdio.ts @@ -3,6 +3,16 @@ import { JSONRPCMessageSchema } from '../types/index'; export const STDIO_DEFAULT_MAX_BUFFER_SIZE = 10 * 1024 * 1024; +export class InvalidJsonRpcFrameError extends Error { + readonly rawFrame: unknown; + + constructor(rawFrame: unknown, cause: unknown) { + super('Invalid JSON-RPC message', { cause }); + this.name = 'InvalidJsonRpcFrameError'; + this.rawFrame = rawFrame; + } +} + /** * Buffers a continuous stdio stream into discrete JSON-RPC messages. */ @@ -54,7 +64,12 @@ export class ReadBuffer { } export function deserializeMessage(line: string): JSONRPCMessage { - return JSONRPCMessageSchema.parse(JSON.parse(line)); + const rawFrame = JSON.parse(line); + const parsed = JSONRPCMessageSchema.safeParse(rawFrame); + if (!parsed.success) { + throw new InvalidJsonRpcFrameError(rawFrame, parsed.error); + } + return parsed.data; } export function serializeMessage(message: JSONRPCMessage): string { diff --git a/packages/core-internal/test/shared/stdio.test.ts b/packages/core-internal/test/shared/stdio.test.ts index f8d27a4c1f..e427ef821c 100644 --- a/packages/core-internal/test/shared/stdio.test.ts +++ b/packages/core-internal/test/shared/stdio.test.ts @@ -1,4 +1,4 @@ -import { ReadBuffer, STDIO_DEFAULT_MAX_BUFFER_SIZE } from '../../src/shared/stdio'; +import { deserializeMessage, InvalidJsonRpcFrameError, ReadBuffer, STDIO_DEFAULT_MAX_BUFFER_SIZE } from '../../src/shared/stdio'; import type { JSONRPCMessage } from '../../src/types/index'; const testMessage: JSONRPCMessage = { @@ -110,7 +110,20 @@ describe('non-JSON line filtering', () => { const readBuffer = new ReadBuffer(); readBuffer.append(Buffer.from('{"not": "a jsonrpc message"}\n')); - expect(() => readBuffer.readMessage()).toThrow(); + expect(() => readBuffer.readMessage()).toThrow(InvalidJsonRpcFrameError); + }); + + test('should preserve the parsed frame when valid JSON fails schema validation', () => { + const rawFrame = { id: 99, method: 'tools/list', params: {} }; + + expect(() => deserializeMessage(JSON.stringify(rawFrame))).toThrow(InvalidJsonRpcFrameError); + + try { + deserializeMessage(JSON.stringify(rawFrame)); + } catch (error) { + expect(error).toBeInstanceOf(InvalidJsonRpcFrameError); + expect((error as InvalidJsonRpcFrameError).rawFrame).toEqual(rawFrame); + } }); }); diff --git a/packages/server/src/server/stdio.ts b/packages/server/src/server/stdio.ts index e8fda03257..c8945a228d 100644 --- a/packages/server/src/server/stdio.ts +++ b/packages/server/src/server/stdio.ts @@ -1,9 +1,24 @@ import type { Readable, Writable } from 'node:stream'; -import type { JSONRPCMessage, Transport } from '@modelcontextprotocol/core-internal'; -import { ReadBuffer, serializeMessage } from '@modelcontextprotocol/core-internal'; +import type { JSONRPCMessage, RequestId, Transport } from '@modelcontextprotocol/core-internal'; +import { + INVALID_REQUEST, + InvalidJsonRpcFrameError, + JSONRPC_VERSION, + ReadBuffer, + serializeMessage +} from '@modelcontextprotocol/core-internal'; import { process } from '@modelcontextprotocol/server/_shims'; +interface InvalidRequestResponse { + jsonrpc: typeof JSONRPC_VERSION; + id: RequestId | null; + error: { + code: typeof INVALID_REQUEST; + message: 'Invalid Request'; + }; +} + /** * Server transport for stdio: this communicates with an MCP client by reading from the current process' `stdin` and writing to `stdout`. * @@ -42,10 +57,10 @@ export class StdioServerTransport implements Transport { onmessage?: (message: JSONRPCMessage) => void; // Arrow functions to bind `this` properly, while maintaining function identity. - _ondata = (chunk: Buffer) => { + _ondata = async (chunk: Buffer) => { try { this._readBuffer.append(chunk); - this.processReadBuffer(); + await this.processReadBuffer(); } catch (error) { this.onerror?.(error as Error); this.close().catch(() => {}); @@ -77,7 +92,7 @@ export class StdioServerTransport implements Transport { this._stdout.on('error', this._onstdouterror); } - private processReadBuffer() { + private async processReadBuffer(): Promise { while (true) { try { const message = this._readBuffer.readMessage(); @@ -87,11 +102,29 @@ export class StdioServerTransport implements Transport { this.onmessage?.(message); } catch (error) { + if (error instanceof InvalidJsonRpcFrameError) { + this.onerror?.(error); + await this.sendInvalidRequestResponse(error); + continue; + } this.onerror?.(error as Error); } } } + private async sendInvalidRequestResponse(error: InvalidJsonRpcFrameError): Promise { + const response: InvalidRequestResponse = { + jsonrpc: JSONRPC_VERSION, + id: recoverRequestId(error.rawFrame), + error: { + code: INVALID_REQUEST, + message: 'Invalid Request' + } + }; + + await this.writeSerializedMessage(JSON.stringify(response) + '\n'); + } + async close(): Promise { if (this._closed) { return; @@ -120,9 +153,11 @@ export class StdioServerTransport implements Transport { if (this._closed) { return Promise.reject(new Error('StdioServerTransport is closed')); } - return new Promise((resolve, reject) => { - const json = serializeMessage(message); + return this.writeSerializedMessage(serializeMessage(message)); + } + private writeSerializedMessage(json: string): Promise { + return new Promise((resolve, reject) => { let settled = false; const onError = (error: Error) => { if (settled) return; @@ -152,3 +187,16 @@ export class StdioServerTransport implements Transport { }); } } + +function recoverRequestId(rawFrame: unknown): RequestId | null { + if (typeof rawFrame !== 'object' || rawFrame === null || Array.isArray(rawFrame)) { + return null; + } + + const id = (rawFrame as { id?: unknown }).id; + if (typeof id === 'string' || (typeof id === 'number' && Number.isInteger(id))) { + return id; + } + + return null; +} diff --git a/packages/server/test/server/stdio.test.ts b/packages/server/test/server/stdio.test.ts index fe79e3679c..ab9a79ce27 100644 --- a/packages/server/test/server/stdio.test.ts +++ b/packages/server/test/server/stdio.test.ts @@ -1,12 +1,12 @@ import { Readable, Writable } from 'node:stream'; import type { JSONRPCMessage } from '@modelcontextprotocol/core-internal'; -import { ReadBuffer, serializeMessage } from '@modelcontextprotocol/core-internal'; +import { INVALID_REQUEST, InvalidJsonRpcFrameError, serializeMessage } from '@modelcontextprotocol/core-internal'; import { StdioServerTransport } from '../../src/server/stdio'; let input: Readable; -let outputBuffer: ReadBuffer; +let outputChunks: Buffer[]; let output: Writable; beforeEach(() => { @@ -15,15 +15,27 @@ beforeEach(() => { read: () => {} }); - outputBuffer = new ReadBuffer(); + outputChunks = []; output = new Writable({ write(chunk, _encoding, callback) { - outputBuffer.append(chunk); + outputChunks.push(Buffer.from(chunk)); callback(); } }); }); +function readOutputMessages(): unknown[] { + const output = Buffer.concat(outputChunks).toString('utf8').trim(); + if (output.length === 0) { + return []; + } + return output.split('\n').map(line => JSON.parse(line)); +} + +async function flushEvents(): Promise { + await new Promise(resolve => setImmediate(resolve)); +} + test('should start then close cleanly', async () => { const server = new StdioServerTransport(input, output); server.onerror = error => { @@ -103,6 +115,50 @@ test('should read multiple messages', async () => { expect(readMessages).toEqual(messages); }); +test('should reply with Invalid Request when a malformed JSON-RPC request has a recoverable id', async () => { + const server = new StdioServerTransport(input, output); + const errors: Error[] = []; + server.onerror = error => { + errors.push(error); + }; + + await server.start(); + input.push('{"id":99,"method":"tools/list","params":{}}\n'); + await flushEvents(); + + expect(errors).toHaveLength(1); + expect(errors[0]).toBeInstanceOf(InvalidJsonRpcFrameError); + expect(readOutputMessages()).toEqual([ + { + jsonrpc: '2.0', + id: 99, + error: { code: INVALID_REQUEST, message: 'Invalid Request' } + } + ]); +}); + +test('should reply with null id when a malformed JSON-RPC frame has no recoverable id', async () => { + const server = new StdioServerTransport(input, output); + const errors: Error[] = []; + server.onerror = error => { + errors.push(error); + }; + + await server.start(); + input.push('[{"jsonrpc":"2.0","id":100,"method":"tools/list"},{"jsonrpc":"2.0","id":101,"method":"ping"}]\n'); + await flushEvents(); + + expect(errors).toHaveLength(1); + expect(errors[0]).toBeInstanceOf(InvalidJsonRpcFrameError); + expect(readOutputMessages()).toEqual([ + { + jsonrpc: '2.0', + id: null, + error: { code: INVALID_REQUEST, message: 'Invalid Request' } + } + ]); +}); + test('should close and fire onerror when stdout errors', async () => { const server = new StdioServerTransport(input, output);