Skip to content

Commit 15a556f

Browse files
committed
fix: reject overflowing keep-alive intervals
Reject timer delays above Node's maximum in addition to NaN and Infinity, and keep the lifecycle regression focused on cleanup rather than the transport's pre-existing error-status mapping.
1 parent 739c009 commit 15a556f

2 files changed

Lines changed: 8 additions & 10 deletions

File tree

src/server/webStandardStreamableHttp.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,9 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
274274
private startKeepAlive(streamId: string, controller: ReadableStreamDefaultController<Uint8Array>, encoder: TextEncoder): void {
275275
// A deferred arm (e.g. after an event-store await that straddled
276276
// close()) must not outlive the transport: close()'s timer sweep has
277-
// already run. Non-finite intervals (NaN, Infinity) and non-positive
278-
// ones disable keep-alive: setInterval would clamp them to ~1ms and
279-
// flood every stream with comment frames.
280-
if (!Number.isFinite(this._keepAliveMs) || this._keepAliveMs <= 0 || this._closed) {
277+
// already run. Invalid timer delays disable keep-alive rather than
278+
// 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) {
281280
return;
282281
}
283282
this.stopKeepAlive(streamId);
@@ -681,8 +680,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
681680
private async handlePostRequest(req: Request, options?: HandleRequestOptions): Promise<Response> {
682681
// Set once the SSE stream bookkeeping has been registered, so the
683682
// catch below can reclaim it: an error after registration (a failed
684-
// priming event write, a throwing message handler) returns 400 and
685-
// discards the Response, leaving nothing that could ever cancel the
683+
// priming event write, a throwing message handler) returns an error
684+
// response, leaving nothing that could ever cancel the discarded
686685
// stream or retire the request mappings.
687686
let reclaimSseBookkeeping: (() => void) | undefined;
688687
try {

test/server/streamableHttp.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3452,8 +3452,8 @@ describe('WebStandardStreamableHTTPServerTransport SSE keep-alive', () => {
34523452
await transport.close();
34533453
});
34543454

3455-
it.each([Number.NaN, Number.POSITIVE_INFINITY])(
3456-
'should disable keep-alive for non-finite keepAliveMs %s instead of arming a clamped interval',
3455+
it.each([Number.NaN, Number.POSITIVE_INFINITY, 2_147_483_648])(
3456+
'should disable keep-alive for invalid keepAliveMs %s instead of arming a clamped interval',
34573457
async keepAliveMs => {
34583458
const { transport, sessionId } = await createTransport({ keepAliveMs });
34593459

@@ -3542,13 +3542,12 @@ describe('WebStandardStreamableHTTPServerTransport SSE keep-alive', () => {
35423542
expect(vi.getTimerCount()).toBe(0);
35433543
storeFails = true;
35443544

3545-
const response = await transport.handleRequest(
3545+
await transport.handleRequest(
35463546
req('POST', {
35473547
body: { jsonrpc: '2.0', method: 'tools/call', params: { name: 'noop', arguments: {} }, id: 'call-1' },
35483548
headers: withSession(sessionId)
35493549
})
35503550
);
3551-
expect(response.status).toBe(400);
35523551

35533552
// The discarded stream must not carry a permanently-firing timer
35543553
expect(vi.getTimerCount()).toBe(0);

0 commit comments

Comments
 (0)