Skip to content

Commit e7ee57c

Browse files
v1 stdio buffer limit (#2239)
1 parent c36e1ef commit e7ee57c

6 files changed

Lines changed: 183 additions & 9 deletions

File tree

src/client/stdio.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ export type StdioServerParameters = {
3737
* If not specified, the current working directory will be inherited.
3838
*/
3939
cwd?: string;
40+
41+
/**
42+
* Maximum size of the read buffer in bytes. If a single message exceeds
43+
* this size the transport will emit an error and close.
44+
*
45+
* Defaults to 10 MB.
46+
*/
47+
maxBufferSize?: number;
4048
};
4149

4250
/**
@@ -91,7 +99,7 @@ export function getDefaultEnvironment(): Record<string, string> {
9199
*/
92100
export class StdioClientTransport implements Transport {
93101
private _process?: ChildProcess;
94-
private _readBuffer: ReadBuffer = new ReadBuffer();
102+
private _readBuffer: ReadBuffer;
95103
private _serverParams: StdioServerParameters;
96104
private _stderrStream: PassThrough | null = null;
97105

@@ -101,6 +109,7 @@ export class StdioClientTransport implements Transport {
101109

102110
constructor(server: StdioServerParameters) {
103111
this._serverParams = server;
112+
this._readBuffer = new ReadBuffer({ maxBufferSize: server.maxBufferSize });
104113
if (server.stderr === 'pipe' || server.stderr === 'overlapped') {
105114
this._stderrStream = new PassThrough();
106115
}
@@ -148,8 +157,13 @@ export class StdioClientTransport implements Transport {
148157
});
149158

150159
this._process.stdout?.on('data', chunk => {
151-
this._readBuffer.append(chunk);
152-
this.processReadBuffer();
160+
try {
161+
this._readBuffer.append(chunk);
162+
this.processReadBuffer();
163+
} catch (error) {
164+
this.onerror?.(error as Error);
165+
this.close().catch(() => {});
166+
}
153167
});
154168

155169
this._process.stdout?.on('error', error => {

src/server/stdio.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,38 @@ import { Transport } from '../shared/transport.js';
1010
* This transport is only available in Node.js environments.
1111
*/
1212
export class StdioServerTransport implements Transport {
13-
private _readBuffer: ReadBuffer = new ReadBuffer();
13+
private _readBuffer: ReadBuffer;
1414
private _started = false;
1515

1616
constructor(
1717
private _stdin: Readable = process.stdin,
18-
private _stdout: Writable = process.stdout
19-
) {}
18+
private _stdout: Writable = process.stdout,
19+
options?: {
20+
/**
21+
* Maximum size of the read buffer in bytes. If a single message exceeds
22+
* this size the transport will emit an error and close.
23+
*
24+
* Defaults to 10 MB.
25+
*/
26+
maxBufferSize?: number;
27+
}
28+
) {
29+
this._readBuffer = new ReadBuffer({ maxBufferSize: options?.maxBufferSize });
30+
}
2031

2132
onclose?: () => void;
2233
onerror?: (error: Error) => void;
2334
onmessage?: (message: JSONRPCMessage) => void;
2435

2536
// Arrow functions to bind `this` properly, while maintaining function identity.
2637
_ondata = (chunk: Buffer) => {
27-
this._readBuffer.append(chunk);
28-
this.processReadBuffer();
38+
try {
39+
this._readBuffer.append(chunk);
40+
this.processReadBuffer();
41+
} catch (error) {
42+
this.onerror?.(error as Error);
43+
this.close().catch(() => {});
44+
}
2945
};
3046
_onerror = (error: Error) => {
3147
this.onerror?.(error);

src/shared/stdio.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
11
import { JSONRPCMessage, JSONRPCMessageSchema } from '../types.js';
22

3+
export const STDIO_DEFAULT_MAX_BUFFER_SIZE = 10 * 1024 * 1024;
4+
35
/**
46
* Buffers a continuous stdio stream into discrete JSON-RPC messages.
57
*/
68
export class ReadBuffer {
79
private _buffer?: Buffer;
10+
private _maxBufferSize: number;
11+
12+
constructor(options?: { maxBufferSize?: number }) {
13+
this._maxBufferSize = options?.maxBufferSize ?? STDIO_DEFAULT_MAX_BUFFER_SIZE;
14+
}
815

916
append(chunk: Buffer): void {
17+
const newSize = (this._buffer?.length ?? 0) + chunk.length;
18+
if (newSize > this._maxBufferSize) {
19+
this.clear();
20+
throw new Error(`ReadBuffer exceeded maximum size of ${this._maxBufferSize} bytes`);
21+
}
1022
this._buffer = this._buffer ? Buffer.concat([this._buffer, chunk]) : chunk;
1123
}
1224

test/client/stdio.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,44 @@ test('should return child process pid', async () => {
7575
await client.close();
7676
expect(client.pid).toBeNull();
7777
});
78+
79+
test('should respect custom maxBufferSize option', async () => {
80+
const client = new StdioClientTransport({
81+
command: 'node',
82+
args: ['-e', 'process.stdout.write(Buffer.alloc(200, 0x41))'],
83+
maxBufferSize: 100
84+
});
85+
86+
const errorReceived = new Promise<Error>(resolve => {
87+
client.onerror = resolve;
88+
});
89+
const closed = new Promise<void>(resolve => {
90+
client.onclose = () => resolve();
91+
});
92+
93+
await client.start();
94+
95+
const error = await errorReceived;
96+
expect(error.message).toMatch(/ReadBuffer exceeded maximum size/);
97+
await closed;
98+
});
99+
100+
test('should fire onerror and close when ReadBuffer overflows', async () => {
101+
const client = new StdioClientTransport({
102+
command: 'node',
103+
args: ['-e', 'process.stdout.write(Buffer.alloc(11 * 1024 * 1024, 0x41))']
104+
});
105+
106+
const errorReceived = new Promise<Error>(resolve => {
107+
client.onerror = resolve;
108+
});
109+
const closed = new Promise<void>(resolve => {
110+
client.onclose = () => resolve();
111+
});
112+
113+
await client.start();
114+
115+
const error = await errorReceived;
116+
expect(error.message).toMatch(/ReadBuffer exceeded maximum size/);
117+
await closed;
118+
});

test/server/stdio.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,51 @@ test('should read multiple messages', async () => {
100100
await finished;
101101
expect(readMessages).toEqual(messages);
102102
});
103+
104+
test('should respect custom maxBufferSize option', async () => {
105+
const server = new StdioServerTransport(input, output, { maxBufferSize: 100 });
106+
107+
let receivedError: Error | undefined;
108+
server.onerror = err => {
109+
receivedError = err;
110+
};
111+
let closeCount = 0;
112+
server.onclose = () => {
113+
closeCount++;
114+
};
115+
116+
await server.start();
117+
118+
// Push 101 bytes without a newline — exceeds the 100-byte limit
119+
input.push(Buffer.alloc(101, 0x41));
120+
121+
await new Promise(resolve => setTimeout(resolve, 10));
122+
123+
expect(receivedError?.message).toMatch(/ReadBuffer exceeded maximum size/);
124+
expect(closeCount).toBe(1);
125+
});
126+
127+
test('should fire onerror and close when ReadBuffer overflows', async () => {
128+
const server = new StdioServerTransport(input, output);
129+
130+
let receivedError: Error | undefined;
131+
server.onerror = err => {
132+
receivedError = err;
133+
};
134+
let closeCount = 0;
135+
server.onclose = () => {
136+
closeCount++;
137+
};
138+
139+
await server.start();
140+
141+
// Push data exceeding the default 10 MB limit without a newline
142+
const chunk = Buffer.alloc(11 * 1024 * 1024, 0x41);
143+
input.push(chunk);
144+
145+
// Allow the close() promise to settle
146+
await new Promise(resolve => setTimeout(resolve, 10));
147+
148+
expect(receivedError?.message).toMatch(/ReadBuffer exceeded maximum size/);
149+
expect(closeCount).toBe(1);
150+
});

test/shared/stdio.test.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { JSONRPCMessage } from '../../src/types.js';
2-
import { ReadBuffer } from '../../src/shared/stdio.js';
2+
import { STDIO_DEFAULT_MAX_BUFFER_SIZE, ReadBuffer } from '../../src/shared/stdio.js';
33

44
const testMessage: JSONRPCMessage = {
55
jsonrpc: '2.0',
@@ -33,3 +33,46 @@ test('should be reusable after clearing', () => {
3333
readBuffer.append(Buffer.from('\n'));
3434
expect(readBuffer.readMessage()).toEqual(testMessage);
3535
});
36+
37+
describe('buffer size limit', () => {
38+
test('should throw when buffer exceeds default max size', () => {
39+
const readBuffer = new ReadBuffer();
40+
const chunkSize = 1024 * 1024; // 1 MB
41+
const chunk = Buffer.alloc(chunkSize);
42+
const chunksToFill = Math.floor(STDIO_DEFAULT_MAX_BUFFER_SIZE / chunkSize);
43+
for (let i = 0; i < chunksToFill; i++) {
44+
readBuffer.append(chunk);
45+
}
46+
expect(() => readBuffer.append(chunk)).toThrow(/ReadBuffer exceeded maximum size/);
47+
});
48+
49+
test('should throw when buffer exceeds custom max size', () => {
50+
const readBuffer = new ReadBuffer({ maxBufferSize: 100 });
51+
readBuffer.append(Buffer.alloc(50));
52+
expect(() => readBuffer.append(Buffer.alloc(51))).toThrow(/ReadBuffer exceeded maximum size/);
53+
});
54+
55+
test('should clear buffer before throwing on overflow', () => {
56+
const readBuffer = new ReadBuffer({ maxBufferSize: 100 });
57+
readBuffer.append(Buffer.alloc(50));
58+
expect(() => readBuffer.append(Buffer.alloc(51))).toThrow();
59+
60+
// Buffer should be cleared — can append again
61+
readBuffer.append(Buffer.alloc(50));
62+
// And read messages normally
63+
expect(readBuffer.readMessage()).toBeNull();
64+
});
65+
66+
test('should allow appending up to exactly the max size', () => {
67+
const readBuffer = new ReadBuffer({ maxBufferSize: 100 });
68+
// Should not throw — exactly at limit
69+
expect(() => readBuffer.append(Buffer.alloc(100))).not.toThrow();
70+
});
71+
72+
test('should work with no options (backwards compatible)', () => {
73+
const readBuffer = new ReadBuffer();
74+
// Small append should always work
75+
readBuffer.append(Buffer.from(JSON.stringify({ jsonrpc: '2.0', method: 'ping' }) + '\n'));
76+
expect(readBuffer.readMessage()).not.toBeNull();
77+
});
78+
});

0 commit comments

Comments
 (0)