From f15933abca1f545079e3f6d171d40773258046d0 Mon Sep 17 00:00:00 2001 From: Neil Daquioag Date: Sat, 18 Apr 2026 06:52:44 +0800 Subject: [PATCH 1/2] fix(request): linear SSE buffering with pre-append size check Addresses REQ-HIGH-03 deep-audit finding. Previous: fullText += decoder.decode(...) caused O(n^2) string concatenation in convertSseToJson; MAX_SSE_SIZE check ran AFTER append so memory briefly held chunk + 10MB before throwing. Fix: accumulate chunks in string[] array; track running size; check size BEFORE append; final join() once at end. Linear time, bounded memory, size-check enforced pre-allocation. Test asserts pre-append throw when next chunk would exceed cap. --- lib/request/response-handler.ts | 19 +++- test/response-handler-sse-buffer.test.ts | 115 +++++++++++++++++++++++ 2 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 test/response-handler-sse-buffer.test.ts diff --git a/lib/request/response-handler.ts b/lib/request/response-handler.ts index fc13886e7..786827b7a 100644 --- a/lib/request/response-handler.ts +++ b/lib/request/response-handler.ts @@ -766,7 +766,13 @@ export async function convertSseToJson( } const reader = response.body.getReader(); const decoder = new TextDecoder(); - let fullText = ""; + // REQ-HIGH-03: Accumulate decoded chunks in an array instead of concatenating + // into a growing string. Repeated `fullText += chunk` is O(n) per append, + // producing O(n^2) total work on V8 for large streams. The array-then-join() + // pattern is O(n) total. The size check runs BEFORE appending so we never + // allocate a chunk that would exceed MAX_SSE_SIZE. + const chunks: string[] = []; + let totalSize = 0; const streamStallTimeoutMs = Math.max( 1_000, Math.floor( @@ -782,12 +788,19 @@ export async function convertSseToJson( streamStallTimeoutMs, ); if (done) break; - fullText += decoder.decode(value, { stream: true }); - if (fullText.length > MAX_SSE_SIZE) { + const decoded = decoder.decode(value, { stream: true }); + // Pre-append size check: reject before allocating/retaining the chunk + // alongside the accumulated buffer. This bounds peak memory to the + // cap rather than cap + chunk. + if (totalSize + decoded.length > MAX_SSE_SIZE) { throw new Error(`SSE response exceeds ${MAX_SSE_SIZE} bytes limit`); } + chunks.push(decoded); + totalSize += decoded.length; } + const fullText = chunks.join(""); + if (LOGGING_ENABLED) { logRequest("stream-full", { fullContent: fullText }); } diff --git a/test/response-handler-sse-buffer.test.ts b/test/response-handler-sse-buffer.test.ts new file mode 100644 index 000000000..77096ece7 --- /dev/null +++ b/test/response-handler-sse-buffer.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, vi } from "vitest"; +import { convertSseToJson } from "../lib/request/response-handler.js"; + +// REQ-HIGH-03: Linear SSE buffering with pre-append size check. +// +// Previous implementation of convertSseToJson used +// fullText += decoder.decode(value, { stream: true }) +// which is O(n) per append on V8 (string rope materialization), producing +// O(n^2) total work on large SSE streams. It also checked MAX_SSE_SIZE +// AFTER appending, so peak memory transiently held chunk_size + 10MB +// before throwing. +// +// Fix accumulates decoded chunks in a string[] and joins once at the end, +// and enforces the MAX_SSE_SIZE cap BEFORE appending each chunk, so memory +// is bounded to the cap and the throw cannot allocate a cap+chunk buffer. +// +// These tests live in a dedicated file to isolate the regression scenarios +// from the much larger response-handler.test.ts suite. + +describe("convertSseToJson: REQ-HIGH-03 linear buffering with pre-append cap", () => { + // MAX_SSE_SIZE is an internal constant in lib/request/response-handler.ts. + // Mirror it here so tests read the same numeric cap the implementation + // enforces. Drift is caught by the "just under cap" test. + const MAX_SSE_SIZE_FOR_TEST = 10 * 1024 * 1024; // 10MB, must match lib constant + + // Build a multi-chunk SSE stream reader that emits ASCII bytes. Using + // ASCII guarantees decoder.decode(value, { stream: true }) returns a + // string of the same length as the byte buffer, so totalSize tracking + // is deterministic relative to MAX_SSE_SIZE (10MB). + const buildChunkedReader = (chunks: Uint8Array[]) => { + let i = 0; + const reader = { + read: vi.fn(async () => { + if (i >= chunks.length) return { done: true, value: undefined }; + const value = chunks[i++]; + return { done: false, value }; + }), + cancel: vi.fn(async () => undefined), + releaseLock: vi.fn(), + }; + const response = { + body: { getReader: () => reader }, + status: 200, + statusText: "OK", + } as unknown as Response; + return { response, reader }; + }; + + it("accumulates many chunks totaling just under the 10MB cap without throwing on append", async () => { + // Build a valid SSE response whose payload is large (~9MB of filler + // inside a JSON string field plus a terminal done event), split + // across many ~128KB chunks. The sum stays below the 10MB cap so + // the pre-append check must not trigger, and all chunks accumulate + // and parse correctly. + const filler = "x".repeat(9 * 1024 * 1024); + const sse = + 'data: {"type":"response.started"}\n' + + `data: {"type":"response.done","response":{"id":"resp_big","output":${JSON.stringify(filler)}}}\n`; + const bytes = new TextEncoder().encode(sse); + expect(bytes.byteLength).toBeLessThan(MAX_SSE_SIZE_FOR_TEST); + + const chunkSize = 128 * 1024; + const chunks: Uint8Array[] = []; + for (let off = 0; off < bytes.byteLength; off += chunkSize) { + chunks.push( + bytes.slice(off, Math.min(off + chunkSize, bytes.byteLength)), + ); + } + expect(chunks.length).toBeGreaterThan(1); + + const { response, reader } = buildChunkedReader(chunks); + + const result = await convertSseToJson(response, new Headers()); + const body = (await result.json()) as { id: string; output: string }; + expect(body.id).toBe("resp_big"); + expect(body.output.length).toBe(filler.length); + // One read per chunk plus the final done=true read. + expect(reader.read).toHaveBeenCalledTimes(chunks.length + 1); + }); + + it("throws on the FIRST chunk that would exceed the cap (pre-append, not post-append)", async () => { + // Chunk 1: stays safely under the cap. + // Chunk 2: small on its own, but pushes totalSize past MAX_SSE_SIZE. + // The old implementation would append first then throw after the + // accumulated string had already grown past the cap. The fix must + // throw BEFORE appending chunk 2, so the reader is only called for + // the two chunks and the second chunk is never retained alongside + // the accumulated buffer. + const nearCap = new Uint8Array(MAX_SSE_SIZE_FOR_TEST - 16).fill(0x61); // 'a' + const overflow = new Uint8Array(64).fill(0x62); // 'b' + const { response, reader } = buildChunkedReader([nearCap, overflow]); + + await expect(convertSseToJson(response, new Headers())).rejects.toThrow( + /exceeds.*bytes limit/, + ); + + // Two reads: first chunk accepted, second chunk checked pre-append + // and rejected. No further reads after throw. + expect(reader.read).toHaveBeenCalledTimes(2); + expect(reader.cancel).toHaveBeenCalled(); + }); + + it("throws on the very first chunk when it alone exceeds the cap", async () => { + // Single chunk that blows the cap by itself. Fix must reject before + // attempting to push/retain it alongside an empty buffer. + const tooBig = new Uint8Array(MAX_SSE_SIZE_FOR_TEST + 1).fill(0x63); // 'c' + const { response, reader } = buildChunkedReader([tooBig]); + + await expect(convertSseToJson(response, new Headers())).rejects.toThrow( + /exceeds.*bytes limit/, + ); + expect(reader.read).toHaveBeenCalledTimes(1); + expect(reader.cancel).toHaveBeenCalled(); + }); +}); From afd1fffb2f91baf25b8693fe57838b19908ee2f1 Mon Sep 17 00:00:00 2001 From: Neil Daquioag Date: Sat, 18 Apr 2026 16:02:13 +0800 Subject: [PATCH 2/2] fix(request): count utf-8 bytes in SSE size guard Responds to PR #418 review feedback. The pre-append SSE size guard used decoded.length, which counts UTF-16 code units rather than bytes. Multi-byte UTF-8 chunks (emoji, many CJK chars) could therefore exceed MAX_SSE_SIZE without tripping the guard at the right point. Use Buffer.byteLength(decoded, 'utf8') for both the pre-append check and the running total, and add a regression test covering a multi-byte payload. --- lib/request/response-handler.ts | 5 +++-- test/response-handler-sse-buffer.test.ts | 27 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/request/response-handler.ts b/lib/request/response-handler.ts index 786827b7a..5a5724157 100644 --- a/lib/request/response-handler.ts +++ b/lib/request/response-handler.ts @@ -789,14 +789,15 @@ export async function convertSseToJson( ); if (done) break; const decoded = decoder.decode(value, { stream: true }); + const decodedBytes = Buffer.byteLength(decoded, "utf8"); // Pre-append size check: reject before allocating/retaining the chunk // alongside the accumulated buffer. This bounds peak memory to the // cap rather than cap + chunk. - if (totalSize + decoded.length > MAX_SSE_SIZE) { + if (totalSize + decodedBytes > MAX_SSE_SIZE) { throw new Error(`SSE response exceeds ${MAX_SSE_SIZE} bytes limit`); } chunks.push(decoded); - totalSize += decoded.length; + totalSize += decodedBytes; } const fullText = chunks.join(""); diff --git a/test/response-handler-sse-buffer.test.ts b/test/response-handler-sse-buffer.test.ts index 77096ece7..423ff0bba 100644 --- a/test/response-handler-sse-buffer.test.ts +++ b/test/response-handler-sse-buffer.test.ts @@ -112,4 +112,31 @@ describe("convertSseToJson: REQ-HIGH-03 linear buffering with pre-append cap", ( expect(reader.read).toHaveBeenCalledTimes(1); expect(reader.cancel).toHaveBeenCalled(); }); + + it("counts utf-8 bytes rather than utf-16 code units in the pre-append guard", async () => { + // 😀 is 2 UTF-16 code units but 4 UTF-8 bytes. The guard must count bytes + // or it underestimates multi-byte chunks and can exceed the 10MB budget. + const emoji = "😀"; + const repeats = Math.ceil((MAX_SSE_SIZE_FOR_TEST + 16) / 4); + const filler = emoji.repeat(repeats); + const sse = + 'data: {"type":"response.started"}\n' + + `data: {"type":"response.done","response":{"id":"resp_emoji","output":${JSON.stringify(filler)}}}\n`; + const bytes = new TextEncoder().encode(sse); + // Sanity check: real bytes exceed cap even though string length alone would + // underestimate the size. + const decodedLengthEstimate = sse.length; + const byteLength = bytes.byteLength; + if (!(byteLength > MAX_SSE_SIZE_FOR_TEST && decodedLengthEstimate < byteLength)) { + throw new Error("test setup invalid: expected utf-8 bytes > utf-16 length"); + } + + const { response, reader } = buildChunkedReader([bytes]); + await expect(convertSseToJson(response, new Headers())).rejects.toThrow( + /exceeds.*bytes limit/, + ); + // One read only; the first chunk is rejected based on byte length. + expect(reader.read).toHaveBeenCalledTimes(1); + expect(reader.cancel).toHaveBeenCalled(); + }); });