Skip to content

Commit cca65fc

Browse files
committed
fix: align SSE keep-alive delivery safeguards
Reject sub-millisecond intervals that Node clamps to 1ms and disable nginx-style buffering on GET, POST, and replay SSE responses so heartbeat frames reach clients.
1 parent 15a556f commit cca65fc

3 files changed

Lines changed: 14 additions & 7 deletions

File tree

.changeset/keepalive-lifecycle-hardening.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
'@modelcontextprotocol/sdk': patch
33
---
44

5-
Hardens the Streamable HTTP server transport's SSE keep-alive lifecycle: keep-alive timers can no longer be armed after the transport closes or leak when a priming event write fails, and a non-finite `keepAliveMs` disables keep-alive instead of arming a clamped ~1ms interval.
5+
Hardens the Streamable HTTP server transport's SSE keep-alive lifecycle: timers can no longer be armed after transport close or leak when a priming event write fails, invalid timer delays safely disable keep-alive, and SSE responses disable nginx-style proxy buffering.

src/server/webStandardStreamableHttp.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ export interface WebStandardStreamableHTTPServerTransportOptions {
156156
*
157157
* Comment frames are ignored by SSE parsers and never surface as messages.
158158
* Defaults to 15000 (per the WHATWG SSE spec recommendation of roughly every
159-
* 15 seconds). Set to 0 to disable keep-alive frames.
159+
* 15 seconds). Set to 0 to disable keep-alive frames; values below 1, above
160+
* 2147483647, or non-finite values also disable the timer.
160161
*/
161162
keepAliveMs?: number;
162163
}
@@ -276,7 +277,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
276277
// close()) must not outlive the transport: close()'s timer sweep has
277278
// already run. Invalid timer delays disable keep-alive rather than
278279
// letting setInterval clamp them to ~1ms and flood every stream.
279-
if (!Number.isFinite(this._keepAliveMs) || this._keepAliveMs <= 0 || this._keepAliveMs > 2_147_483_647 || this._closed) {
280+
if (!Number.isFinite(this._keepAliveMs) || this._keepAliveMs < 1 || this._keepAliveMs > 2_147_483_647 || this._closed) {
280281
return;
281282
}
282283
this.stopKeepAlive(streamId);
@@ -493,7 +494,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
493494
const headers: Record<string, string> = {
494495
'Content-Type': 'text/event-stream',
495496
'Cache-Control': 'no-cache, no-transform',
496-
Connection: 'keep-alive'
497+
Connection: 'keep-alive',
498+
'X-Accel-Buffering': 'no'
497499
};
498500

499501
// After initialization, always include the session ID if we have one
@@ -552,7 +554,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
552554
const headers: Record<string, string> = {
553555
'Content-Type': 'text/event-stream',
554556
'Cache-Control': 'no-cache, no-transform',
555-
Connection: 'keep-alive'
557+
Connection: 'keep-alive',
558+
'X-Accel-Buffering': 'no'
556559
};
557560

558561
if (this.sessionId !== undefined) {
@@ -839,7 +842,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
839842
const headers: Record<string, string> = {
840843
'Content-Type': 'text/event-stream',
841844
'Cache-Control': 'no-cache',
842-
Connection: 'keep-alive'
845+
Connection: 'keep-alive',
846+
'X-Accel-Buffering': 'no'
843847
};
844848

845849
// After initialization, always include the session ID if we have one

test/server/streamableHttp.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ describe.each(zodTestMatrix)('$zodVersionLabel', (entry: ZodMatrixEntry) => {
270270

271271
expect(response.status).toBe(200);
272272
expect(response.headers.get('content-type')).toBe('text/event-stream');
273+
expect(response.headers.get('x-accel-buffering')).toBe('no');
273274
expect(response.headers.get('mcp-session-id')).toBeDefined();
274275
});
275276

@@ -486,6 +487,7 @@ describe.each(zodTestMatrix)('$zodVersionLabel', (entry: ZodMatrixEntry) => {
486487

487488
expect(sseResponse.status).toBe(200);
488489
expect(sseResponse.headers.get('content-type')).toBe('text/event-stream');
490+
expect(sseResponse.headers.get('x-accel-buffering')).toBe('no');
489491

490492
// Send a notification (server-initiated message) that should appear on SSE stream
491493
const notification: JSONRPCMessage = {
@@ -1444,6 +1446,7 @@ describe.each(zodTestMatrix)('$zodVersionLabel', (entry: ZodMatrixEntry) => {
14441446
});
14451447

14461448
expect(reconnectResponse.status).toBe(200);
1449+
expect(reconnectResponse.headers.get('x-accel-buffering')).toBe('no');
14471450

14481451
// Read the replayed notification
14491452
const reconnectReader = reconnectResponse.body?.getReader();
@@ -3452,7 +3455,7 @@ describe('WebStandardStreamableHTTPServerTransport SSE keep-alive', () => {
34523455
await transport.close();
34533456
});
34543457

3455-
it.each([Number.NaN, Number.POSITIVE_INFINITY, 2_147_483_648])(
3458+
it.each([0.5, Number.NaN, Number.POSITIVE_INFINITY, 2_147_483_648])(
34563459
'should disable keep-alive for invalid keepAliveMs %s instead of arming a clamped interval',
34573460
async keepAliveMs => {
34583461
const { transport, sessionId } = await createTransport({ keepAliveMs });

0 commit comments

Comments
 (0)