parent
8d55531dab
commit
1b53a415ea
8 changed files with 191 additions and 10 deletions
7
.changeset/stdio-max-buffer-size.md
Normal file
7
.changeset/stdio-max-buffer-size.md
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
---
|
||||
'@modelcontextprotocol/core': patch
|
||||
'@modelcontextprotocol/client': patch
|
||||
'@modelcontextprotocol/server': patch
|
||||
---
|
||||
|
||||
Add a configurable `maxBufferSize` (default 10 MB) to the stdio transports. When a single message would push the read buffer past the limit, the transport now emits an `onerror` and closes instead of growing the buffer unbounded. Configure via `new StdioClientTransport({ ..., maxBufferSize })` or `new StdioServerTransport(stdin, stdout, { maxBufferSize })`. The default is exported from `@modelcontextprotocol/core` as `STDIO_DEFAULT_MAX_BUFFER_SIZE`.
|
||||
|
|
@ -38,6 +38,14 @@ export type StdioServerParameters = {
|
|||
* If not specified, the current working directory will be inherited.
|
||||
*/
|
||||
cwd?: string;
|
||||
|
||||
/**
|
||||
* Maximum size of the read buffer in bytes. If a single message exceeds
|
||||
* this size the transport will emit an error and close.
|
||||
*
|
||||
* Defaults to 10 MB.
|
||||
*/
|
||||
maxBufferSize?: number;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -92,7 +100,7 @@ export function getDefaultEnvironment(): Record<string, string> {
|
|||
*/
|
||||
export class StdioClientTransport implements Transport {
|
||||
private _process?: ChildProcess;
|
||||
private _readBuffer: ReadBuffer = new ReadBuffer();
|
||||
private _readBuffer: ReadBuffer;
|
||||
private _serverParams: StdioServerParameters;
|
||||
private _stderrStream: PassThrough | null = null;
|
||||
|
||||
|
|
@ -102,6 +110,7 @@ export class StdioClientTransport implements Transport {
|
|||
|
||||
constructor(server: StdioServerParameters) {
|
||||
this._serverParams = server;
|
||||
this._readBuffer = new ReadBuffer({ maxBufferSize: server.maxBufferSize });
|
||||
if (server.stderr === 'pipe' || server.stderr === 'overlapped') {
|
||||
this._stderrStream = new PassThrough();
|
||||
}
|
||||
|
|
@ -149,8 +158,13 @@ export class StdioClientTransport implements Transport {
|
|||
});
|
||||
|
||||
this._process.stdout?.on('data', chunk => {
|
||||
this._readBuffer.append(chunk);
|
||||
this.processReadBuffer();
|
||||
try {
|
||||
this._readBuffer.append(chunk);
|
||||
this.processReadBuffer();
|
||||
} catch (error) {
|
||||
this.onerror?.(error as Error);
|
||||
this.close().catch(() => {});
|
||||
}
|
||||
});
|
||||
|
||||
this._process.stdout?.on('error', error => {
|
||||
|
|
|
|||
|
|
@ -77,3 +77,44 @@ test('should return child process pid', async () => {
|
|||
await client.close();
|
||||
expect(client.pid).toBeNull();
|
||||
});
|
||||
|
||||
test('should respect custom maxBufferSize option', async () => {
|
||||
const client = new StdioClientTransport({
|
||||
command: 'node',
|
||||
args: ['-e', 'process.stdout.write(Buffer.alloc(200, 0x41))'],
|
||||
maxBufferSize: 100
|
||||
});
|
||||
|
||||
const errorReceived = new Promise<Error>(resolve => {
|
||||
client.onerror = resolve;
|
||||
});
|
||||
const closed = new Promise<void>(resolve => {
|
||||
client.onclose = () => resolve();
|
||||
});
|
||||
|
||||
await client.start();
|
||||
|
||||
const error = await errorReceived;
|
||||
expect(error.message).toMatch(/ReadBuffer exceeded maximum size/);
|
||||
await closed;
|
||||
});
|
||||
|
||||
test('should fire onerror and close when ReadBuffer overflows', async () => {
|
||||
const client = new StdioClientTransport({
|
||||
command: 'node',
|
||||
args: ['-e', 'process.stdout.write(Buffer.alloc(11 * 1024 * 1024, 0x41))']
|
||||
});
|
||||
|
||||
const errorReceived = new Promise<Error>(resolve => {
|
||||
client.onerror = resolve;
|
||||
});
|
||||
const closed = new Promise<void>(resolve => {
|
||||
client.onclose = () => resolve();
|
||||
});
|
||||
|
||||
await client.start();
|
||||
|
||||
const error = await errorReceived;
|
||||
expect(error.message).toMatch(/ReadBuffer exceeded maximum size/);
|
||||
await closed;
|
||||
});
|
||||
|
|
|
|||
|
|
@ -53,7 +53,7 @@ export type {
|
|||
export { DEFAULT_REQUEST_TIMEOUT_MSEC } from '../../shared/protocol.js';
|
||||
|
||||
// stdio message framing utilities (for custom transport authors)
|
||||
export { deserializeMessage, ReadBuffer, serializeMessage } from '../../shared/stdio.js';
|
||||
export { deserializeMessage, ReadBuffer, serializeMessage, STDIO_DEFAULT_MAX_BUFFER_SIZE } from '../../shared/stdio.js';
|
||||
|
||||
// Transport types (NOT normalizeHeaders)
|
||||
export type { FetchLike, Transport, TransportSendOptions } from '../../shared/transport.js';
|
||||
|
|
|
|||
|
|
@ -1,13 +1,25 @@
|
|||
import type { JSONRPCMessage } from '../types/index.js';
|
||||
import { JSONRPCMessageSchema } from '../types/index.js';
|
||||
|
||||
export const STDIO_DEFAULT_MAX_BUFFER_SIZE = 10 * 1024 * 1024;
|
||||
|
||||
/**
|
||||
* Buffers a continuous stdio stream into discrete JSON-RPC messages.
|
||||
*/
|
||||
export class ReadBuffer {
|
||||
private _buffer?: Buffer;
|
||||
private _maxBufferSize: number;
|
||||
|
||||
constructor(options?: { maxBufferSize?: number }) {
|
||||
this._maxBufferSize = options?.maxBufferSize ?? STDIO_DEFAULT_MAX_BUFFER_SIZE;
|
||||
}
|
||||
|
||||
append(chunk: Buffer): void {
|
||||
const newSize = (this._buffer?.length ?? 0) + chunk.length;
|
||||
if (newSize > this._maxBufferSize) {
|
||||
this.clear();
|
||||
throw new Error(`ReadBuffer exceeded maximum size of ${this._maxBufferSize} bytes`);
|
||||
}
|
||||
this._buffer = this._buffer ? Buffer.concat([this._buffer, chunk]) : chunk;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { ReadBuffer } from '../../src/shared/stdio.js';
|
||||
import { ReadBuffer, STDIO_DEFAULT_MAX_BUFFER_SIZE } from '../../src/shared/stdio.js';
|
||||
import type { JSONRPCMessage } from '../../src/types/index.js';
|
||||
|
||||
const testMessage: JSONRPCMessage = {
|
||||
|
|
@ -113,3 +113,46 @@ describe('non-JSON line filtering', () => {
|
|||
expect(() => readBuffer.readMessage()).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buffer size limit', () => {
|
||||
test('should throw when buffer exceeds default max size', () => {
|
||||
const readBuffer = new ReadBuffer();
|
||||
const chunkSize = 1024 * 1024; // 1 MB
|
||||
const chunk = Buffer.alloc(chunkSize);
|
||||
const chunksToFill = Math.floor(STDIO_DEFAULT_MAX_BUFFER_SIZE / chunkSize);
|
||||
for (let i = 0; i < chunksToFill; i++) {
|
||||
readBuffer.append(chunk);
|
||||
}
|
||||
expect(() => readBuffer.append(chunk)).toThrow(/ReadBuffer exceeded maximum size/);
|
||||
});
|
||||
|
||||
test('should throw when buffer exceeds custom max size', () => {
|
||||
const readBuffer = new ReadBuffer({ maxBufferSize: 100 });
|
||||
readBuffer.append(Buffer.alloc(50));
|
||||
expect(() => readBuffer.append(Buffer.alloc(51))).toThrow(/ReadBuffer exceeded maximum size/);
|
||||
});
|
||||
|
||||
test('should clear buffer before throwing on overflow', () => {
|
||||
const readBuffer = new ReadBuffer({ maxBufferSize: 100 });
|
||||
readBuffer.append(Buffer.alloc(50));
|
||||
expect(() => readBuffer.append(Buffer.alloc(51))).toThrow();
|
||||
|
||||
// Buffer should be cleared — can append again
|
||||
readBuffer.append(Buffer.alloc(50));
|
||||
// And read messages normally
|
||||
expect(readBuffer.readMessage()).toBeNull();
|
||||
});
|
||||
|
||||
test('should allow appending up to exactly the max size', () => {
|
||||
const readBuffer = new ReadBuffer({ maxBufferSize: 100 });
|
||||
// Should not throw — exactly at limit
|
||||
expect(() => readBuffer.append(Buffer.alloc(100))).not.toThrow();
|
||||
});
|
||||
|
||||
test('should work with no options (backwards compatible)', () => {
|
||||
const readBuffer = new ReadBuffer();
|
||||
// Small append should always work
|
||||
readBuffer.append(Buffer.from(JSON.stringify({ jsonrpc: '2.0', method: 'ping' }) + '\n'));
|
||||
expect(readBuffer.readMessage()).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -17,14 +17,25 @@ import { process } from '@modelcontextprotocol/server/_shims';
|
|||
* ```
|
||||
*/
|
||||
export class StdioServerTransport implements Transport {
|
||||
private _readBuffer: ReadBuffer = new ReadBuffer();
|
||||
private _readBuffer: ReadBuffer;
|
||||
private _started = false;
|
||||
private _closed = false;
|
||||
|
||||
constructor(
|
||||
private _stdin: Readable = process.stdin,
|
||||
private _stdout: Writable = process.stdout
|
||||
) {}
|
||||
private _stdout: Writable = process.stdout,
|
||||
options?: {
|
||||
/**
|
||||
* Maximum size of the read buffer in bytes. If a single message exceeds
|
||||
* this size the transport will emit an error and close.
|
||||
*
|
||||
* Defaults to 10 MB.
|
||||
*/
|
||||
maxBufferSize?: number;
|
||||
}
|
||||
) {
|
||||
this._readBuffer = new ReadBuffer({ maxBufferSize: options?.maxBufferSize });
|
||||
}
|
||||
|
||||
onclose?: () => void;
|
||||
onerror?: (error: Error) => void;
|
||||
|
|
@ -32,8 +43,13 @@ export class StdioServerTransport implements Transport {
|
|||
|
||||
// Arrow functions to bind `this` properly, while maintaining function identity.
|
||||
_ondata = (chunk: Buffer) => {
|
||||
this._readBuffer.append(chunk);
|
||||
this.processReadBuffer();
|
||||
try {
|
||||
this._readBuffer.append(chunk);
|
||||
this.processReadBuffer();
|
||||
} catch (error) {
|
||||
this.onerror?.(error as Error);
|
||||
this.close().catch(() => {});
|
||||
}
|
||||
};
|
||||
_onerror = (error: Error) => {
|
||||
this.onerror?.(error);
|
||||
|
|
|
|||
|
|
@ -179,3 +179,51 @@ test('should fire onerror before onclose on stdout error', async () => {
|
|||
|
||||
expect(events).toEqual(['error', 'close']);
|
||||
});
|
||||
|
||||
test('should respect custom maxBufferSize option', async () => {
|
||||
const server = new StdioServerTransport(input, output, { maxBufferSize: 100 });
|
||||
|
||||
let receivedError: Error | undefined;
|
||||
server.onerror = err => {
|
||||
receivedError = err;
|
||||
};
|
||||
let closeCount = 0;
|
||||
server.onclose = () => {
|
||||
closeCount++;
|
||||
};
|
||||
|
||||
await server.start();
|
||||
|
||||
// Push 101 bytes without a newline — exceeds the 100-byte limit
|
||||
input.push(Buffer.alloc(101, 0x41));
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
|
||||
expect(receivedError?.message).toMatch(/ReadBuffer exceeded maximum size/);
|
||||
expect(closeCount).toBe(1);
|
||||
});
|
||||
|
||||
test('should fire onerror and close when ReadBuffer overflows', async () => {
|
||||
const server = new StdioServerTransport(input, output);
|
||||
|
||||
let receivedError: Error | undefined;
|
||||
server.onerror = err => {
|
||||
receivedError = err;
|
||||
};
|
||||
let closeCount = 0;
|
||||
server.onclose = () => {
|
||||
closeCount++;
|
||||
};
|
||||
|
||||
await server.start();
|
||||
|
||||
// Push data exceeding the default 10 MB limit without a newline
|
||||
const chunk = Buffer.alloc(11 * 1024 * 1024, 0x41);
|
||||
input.push(chunk);
|
||||
|
||||
// Allow the close() promise to settle
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
|
||||
expect(receivedError?.message).toMatch(/ReadBuffer exceeded maximum size/);
|
||||
expect(closeCount).toBe(1);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue