Skip to content

Commit 2d3fe69

Browse files
committed
fix(http): bound the size of buffered JSON responses
response.json() buffers the entire body before parsing with no upper bound, so a large test result --history page or a run with a long steps[] array grows the heap in proportion to the payload. Read the typed JSON response bounded by a configurable cap (HttpClientOptions.maxResponseBytes, default 64 MiB): reject an over-cap Content-Length up front, count bytes while streaming chunked bodies, and throw a typed PAYLOAD_TOO_LARGE (exit 5) with guidance to narrow --page-size / --since.
1 parent 251b593 commit 2d3fe69

2 files changed

Lines changed: 183 additions & 2 deletions

File tree

src/lib/http.test.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ function makeClient(
3333
apiKey?: string | null;
3434
onDebug?: (e: DebugEvent) => void;
3535
onServerVersion?: (info: { minVersion?: string }) => void;
36+
maxResponseBytes?: number;
3637
} = {},
3738
): HttpClient {
3839
const apiKey = 'apiKey' in options ? (options.apiKey ?? undefined) : 'sk-test';
@@ -44,9 +45,61 @@ function makeClient(
4445
random: () => 0,
4546
onDebug: options.onDebug,
4647
onServerVersion: options.onServerVersion,
48+
maxResponseBytes: options.maxResponseBytes,
4749
});
4850
}
4951

52+
describe('response size guard (maxResponseBytes)', () => {
53+
it('rejects a response whose Content-Length exceeds the cap', async () => {
54+
// jsonResponse sets a real Content-Length; a 50-byte cap is well under it.
55+
const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ blob: 'x'.repeat(500) }));
56+
const client = makeClient(fetchImpl as unknown as typeof fetch, { maxResponseBytes: 50 });
57+
58+
const err = await client.get('/tests').catch((e: unknown) => e);
59+
60+
expect(err).toBeInstanceOf(ApiError);
61+
expect((err as ApiError).code).toBe('PAYLOAD_TOO_LARGE');
62+
expect((err as ApiError).exitCode).toBe(5);
63+
expect((err as ApiError).getDetail('maxBytes')).toBe(50);
64+
});
65+
66+
it('rejects an over-cap chunked response that has no Content-Length', async () => {
67+
// A body built from a ReadableStream carries no Content-Length, so only the
68+
// streaming byte-counter can catch it.
69+
const encoder = new TextEncoder();
70+
const stream = new ReadableStream<Uint8Array>({
71+
start(controller) {
72+
controller.enqueue(encoder.encode('{"data":"'));
73+
controller.enqueue(encoder.encode('y'.repeat(500)));
74+
controller.enqueue(encoder.encode('"}'));
75+
controller.close();
76+
},
77+
});
78+
const fetchImpl = vi
79+
.fn()
80+
.mockResolvedValue(
81+
new Response(stream, { status: 200, headers: { 'content-type': 'application/json' } }),
82+
);
83+
const client = makeClient(fetchImpl as unknown as typeof fetch, { maxResponseBytes: 50 });
84+
85+
const err = await client.get('/tests').catch((e: unknown) => e);
86+
87+
expect(err).toBeInstanceOf(ApiError);
88+
expect((err as ApiError).code).toBe('PAYLOAD_TOO_LARGE');
89+
});
90+
91+
it('reads a within-cap response unchanged', async () => {
92+
const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({ ok: true, items: [1, 2, 3] }));
93+
const client = makeClient(fetchImpl as unknown as typeof fetch, {
94+
maxResponseBytes: 1_000_000,
95+
});
96+
97+
const body = await client.get<{ ok: boolean; items: number[] }>('/tests');
98+
99+
expect(body).toEqual({ ok: true, items: [1, 2, 3] });
100+
});
101+
});
102+
50103
describe('CLIENT_TOO_OLD (426)', () => {
51104
it('is not retried — fails fast with the typed error', async () => {
52105
const fetchImpl = vi.fn().mockResolvedValue(errorEnvelopeResponse(426, 'CLIENT_TOO_OLD'));
@@ -717,7 +770,12 @@ describe('HttpClient per-request timeout', () => {
717770
return {
718771
ok: true,
719772
status: 200,
720-
json: () => Promise.reject(timeoutErr),
773+
headers: new Headers(),
774+
body: new ReadableStream<Uint8Array>({
775+
start(controller) {
776+
controller.error(timeoutErr);
777+
},
778+
}),
721779
} as unknown as Response;
722780
});
723781
const client = new HttpClient({

src/lib/http.ts

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ export interface HttpClientOptions {
106106
* `globalShutdown.signal` via the client factory.
107107
*/
108108
shutdownSignal?: AbortSignal;
109+
/**
110+
* Upper bound (bytes) on a successful JSON response body the client will
111+
* buffer before parsing. A response whose declared `Content-Length` — or
112+
* actual streamed size — exceeds this fails fast with a typed
113+
* `PAYLOAD_TOO_LARGE` error instead of growing the heap without limit.
114+
* Defaults to {@link MAX_RESPONSE_BYTES_DEFAULT} (64 MiB).
115+
*/
116+
maxResponseBytes?: number;
109117
}
110118

111119
export interface RequestOptions {
@@ -168,6 +176,15 @@ const MAX_RATE_LIMITED_DELAY_MS = 60_000;
168176
const CONFLICT_DELAY_MS = 1000;
169177
const INTERNAL_DELAY_MS = 500;
170178

179+
// Upper bound on the size of a successful JSON response body the client will
180+
// buffer into memory. `response.json()` reads the ENTIRE body before parsing,
181+
// with no limit, so a large `test result --history` page or a run with a long
182+
// `steps[]` array (getRun `includeSteps`) would grow the heap in proportion to
183+
// the payload. 64 MiB sits far above any legitimate metadata/history/steps
184+
// response yet still bounds a pathological — or hostile — one. Override per
185+
// client via `HttpClientOptions.maxResponseBytes`.
186+
const MAX_RESPONSE_BYTES_DEFAULT = 64 * 1024 * 1024;
187+
171188
/**
172189
* Result of a successful HTTP request, including the parsed body and the
173190
* `x-request-id` that was sent (useful for surfacing in happy-path output).
@@ -189,6 +206,7 @@ export class HttpClient {
189206
private readonly onServerVersion?: (info: { minVersion?: string }) => void;
190207
private readonly requestTimeoutMs: number;
191208
private readonly shutdownSignal?: AbortSignal;
209+
private readonly maxResponseBytes: number;
192210

193211
constructor(options: HttpClientOptions) {
194212
this.baseUrl = trimTrailingSlash(options.baseUrl);
@@ -201,6 +219,7 @@ export class HttpClient {
201219
this.onTransition = options.onTransition;
202220
this.onServerVersion = options.onServerVersion;
203221
this.requestTimeoutMs = options.requestTimeoutMs ?? REQUEST_TIMEOUT_DEFAULT_MS;
222+
this.maxResponseBytes = options.maxResponseBytes ?? MAX_RESPONSE_BYTES_DEFAULT;
204223
}
205224

206225
/**
@@ -595,10 +614,15 @@ export class HttpClient {
595614
durationMs,
596615
});
597616
try {
598-
return { body: (await response.json()) as T, requestId, status: response.status };
617+
const text = await readBoundedText(response, this.maxResponseBytes, requestId);
618+
return { body: JSON.parse(text) as T, requestId, status: response.status };
599619
} catch (err) {
600620
// Interrupt passthrough (see the fetch catch above).
601621
if (err instanceof InterruptError) throw err;
622+
// A bounded-read rejection (PAYLOAD_TOO_LARGE) — or any typed ApiError —
623+
// is a real, actionable outcome; surface it unchanged rather than
624+
// masking it as a malformed-body error below.
625+
if (err instanceof ApiError) throw err;
602626
// A timeout/abort can fire mid-body-read (headers received, stream stalls).
603627
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal);
604628
// Otherwise the successful response body was not valid JSON — a
@@ -919,6 +943,105 @@ export function malformedResponseError(
919943
);
920944
}
921945

946+
/**
947+
* Read a successful response body as text, bounded to `maxBytes`.
948+
*
949+
* `response.json()` buffers the ENTIRE body into memory before parsing, with no
950+
* upper limit — a large `test result --history` page, or a run with a long
951+
* `steps[]` array (getRun `includeSteps`), grows the heap in proportion to the
952+
* payload. This reads the body incrementally and stops once the accumulated
953+
* size crosses `maxBytes`, so a pathological (or hostile) response fails fast
954+
* with a typed PAYLOAD_TOO_LARGE error instead of exhausting memory.
955+
*
956+
* A declared `Content-Length` over the cap is rejected before any body is read.
957+
* Chunked bodies (no `Content-Length`) are bounded by counting bytes as they
958+
* stream. Decoding happens once, at the end, so multi-byte UTF-8 sequences that
959+
* straddle a chunk boundary still decode correctly.
960+
*/
961+
async function readBoundedText(
962+
response: Response,
963+
maxBytes: number,
964+
requestId: string,
965+
): Promise<string> {
966+
const declared = Number(response.headers.get('content-length'));
967+
if (Number.isFinite(declared) && declared > maxBytes) {
968+
throw responseTooLargeError(requestId, maxBytes, declared);
969+
}
970+
const stream = response.body;
971+
if (stream === null) {
972+
// No readable stream exposed (some runtimes / test doubles): fall back to a
973+
// buffered read, then enforce the cap on the materialized text.
974+
const text = await response.text();
975+
if (new TextEncoder().encode(text).length > maxBytes) {
976+
throw responseTooLargeError(requestId, maxBytes);
977+
}
978+
return text;
979+
}
980+
const reader = stream.getReader();
981+
const chunks: Uint8Array[] = [];
982+
let total = 0;
983+
try {
984+
for (;;) {
985+
const { done, value } = await reader.read();
986+
if (done) break;
987+
if (value === undefined) continue;
988+
total += value.byteLength;
989+
if (total > maxBytes) {
990+
await reader.cancel();
991+
throw responseTooLargeError(requestId, maxBytes, total);
992+
}
993+
chunks.push(value);
994+
}
995+
} finally {
996+
try {
997+
reader.releaseLock();
998+
} catch {
999+
// The reader may already be released after cancel(); releasing twice is a
1000+
// no-op we don't want surfacing over the original error.
1001+
}
1002+
}
1003+
return new TextDecoder('utf-8').decode(concatChunks(chunks, total));
1004+
}
1005+
1006+
/** Concatenate byte chunks into a single `Uint8Array` of known total length. */
1007+
function concatChunks(chunks: readonly Uint8Array[], total: number): Uint8Array {
1008+
const out = new Uint8Array(total);
1009+
let offset = 0;
1010+
for (const chunk of chunks) {
1011+
out.set(chunk, offset);
1012+
offset += chunk.byteLength;
1013+
}
1014+
return out;
1015+
}
1016+
1017+
/**
1018+
* Typed error for a response body that exceeds the client-side buffering cap.
1019+
* Reuses PAYLOAD_TOO_LARGE (exit 5, validation family) — the same code the
1020+
* backend returns for oversized request bodies — so machine consumers route on
1021+
* it uniformly. `nextAction` names the knobs that shrink the result set.
1022+
*/
1023+
function responseTooLargeError(
1024+
requestId: string,
1025+
maxBytes: number,
1026+
observedBytes?: number,
1027+
): ApiError {
1028+
const limitMiB = Math.round(maxBytes / (1024 * 1024));
1029+
return new ApiError({
1030+
code: 'PAYLOAD_TOO_LARGE',
1031+
message:
1032+
`The server response exceeded the client-side ${limitMiB} MiB limit and was not ` +
1033+
`buffered, to avoid unbounded memory use.`,
1034+
nextAction:
1035+
'Narrow the result set and retry — e.g. a smaller --page-size, a tighter --since ' +
1036+
'window, or scope --history to a single run.',
1037+
requestId,
1038+
details: {
1039+
maxBytes,
1040+
...(observedBytes !== undefined ? { observedBytes } : {}),
1041+
},
1042+
});
1043+
}
1044+
9221045
export function parseRetryAfter(headerValue: string | null): number | undefined {
9231046
if (!headerValue) return undefined;
9241047
const numeric = Number(headerValue);

0 commit comments

Comments
 (0)