From 80dcef5044c75f74e4f59e83e217d79d69391ffe Mon Sep 17 00:00:00 2001 From: alexeysophia Date: Wed, 19 Aug 2026 20:11:37 +0300 Subject: [PATCH] fix: large base64 attachments on /v1/responses crash the gateway with heap OOM (#126017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(media): stop canonicalizeBase64 allocating one cons-string node per char Per-character append built a rope of ~25 bytes per input character, all live until the final join — a 20 MB base64 attachment transiently needed ~700-900 MB of heap and OOM-crashed the gateway on POST /v1/responses. Validate in the same single pass but collect contiguous non-whitespace runs as slices; already-canonical input is returned unchanged with zero allocations. Measured: 15 MiB attachment 659 MB -> 0 MB transient heap, 4.3 s -> 0.2 s. * fix(media): bound canonicalizeBase64 cleanup memory by input length Review found the run-slicing cleanup unbounded for adversarial input: alternating data characters and whitespace retains one slice object per run (measured 421 MB of heap for an 8 MiB payload shredded to one run per character). Replace the run collection with a single output buffer materialized on the first whitespace and filled in the same validating pass: canonical input still returns unchanged with zero allocations, and any whitespace shape now costs at most one buffer bounded by the input length (measured 0 MB heap delta, 210 ms for the same shredded payload). Adds the many-short-runs regression test the review asked for, guarding both heapUsed and arrayBuffers. * refactor(media): condense canonicalizeBase64 invariant comment Review asked for the repository's 1-3-line invariant form: keep why the buffer is lazy and bounded, drop the implementation-history narration. * chore: retrigger CI (flaky gateway-server shard) * test(media): update base64 memory comment Punchcard-Session: frost-cedar-willow-ae --------- Co-authored-by: Vincent Koc --- packages/media-core/src/base64.test.ts | 73 ++++++++++++++++++++++++++ packages/media-core/src/base64.ts | 56 ++++++++++---------- 2 files changed, 100 insertions(+), 29 deletions(-) diff --git a/packages/media-core/src/base64.test.ts b/packages/media-core/src/base64.test.ts index 22d2e867abb9..298bd54efbae 100644 --- a/packages/media-core/src/base64.test.ts +++ b/packages/media-core/src/base64.test.ts @@ -13,6 +13,49 @@ describe("base64 helpers", () => { expect(canonicalizeBase64(encoded)).toBe(encoded); }); + it("canonicalizeBase64 handles attachment-sized payloads without heap blow-up", () => { + // Regression guard: the previous per-character append built one cons-string + // node per input character (~25 bytes each, all live at once), so this + // 16 MiB payload (21.3 M base64 chars) transiently needed >500 MB of heap. + // The threshold is deliberately generous; the bounded-buffer implementation + // returns already-canonical input unchanged. + const encoded = Buffer.alloc(16 * 1024 * 1024, 0xab).toString("base64"); + const before = process.memoryUsage().heapUsed; + + expect(canonicalizeBase64(encoded)).toBe(encoded); + + const delta = process.memoryUsage().heapUsed - before; + expect(delta).toBeLessThan(100 * 1024 * 1024); + }); + + it("canonicalizeBase64 cleans whitespace inside large payloads", () => { + const encoded = Buffer.alloc(1_000_000, 0xab).toString("base64"); + const wrapped = encoded.replace(/(.{76})/g, "$1\r\n"); + + expect(canonicalizeBase64(wrapped)).toBe(encoded); + }); + + it("canonicalizeBase64 handles one whitespace per character without heap blow-up", () => { + // Worst case for any run-collecting cleanup strategy: every data character + // is its own whitespace-delimited run (2.7 M runs here). The whole cleanup + // must stay bounded by the input length — one output buffer — not by the + // number of runs. + const encoded = Buffer.alloc(2 * 1024 * 1024, 0xab).toString("base64"); + const shredded = encoded.split("").join("\n"); + // heapUsed catches per-run JS objects (slices, rope nodes); arrayBuffers + // catches Buffer-backed strategies — bound both. + const usedBytes = () => { + const usage = process.memoryUsage(); + return usage.heapUsed + usage.arrayBuffers; + }; + const before = usedBytes(); + + expect(canonicalizeBase64(shredded)).toBe(encoded); + + const delta = usedBytes() - before; + expect(delta).toBeLessThan(64 * 1024 * 1024); + }); + it.each([ { name: "canonicalizeBase64 normalizes whitespace and keeps valid base64", @@ -39,6 +82,36 @@ describe("base64 helpers", () => { actual: canonicalizeBase64("ZE=="), expected: undefined, }, + { + name: "canonicalizeBase64 rejects nonzero pad bits on auto-padded input", + actual: canonicalizeBase64("ZE"), + expected: undefined, + }, + { + name: "canonicalizeBase64 trims leading and trailing whitespace", + actual: canonicalizeBase64("\n\tSGVsbG8= "), + expected: "SGVsbG8=", + }, + { + name: "canonicalizeBase64 rejects data chars after padding", + actual: canonicalizeBase64("QQ==QQ=="), + expected: undefined, + }, + { + name: "canonicalizeBase64 rejects more than two padding chars", + actual: canonicalizeBase64("===="), + expected: undefined, + }, + { + name: "canonicalizeBase64 rejects a data: URL prefix", + actual: canonicalizeBase64("data:image/png;base64,QUJD"), + expected: undefined, + }, + { + name: "canonicalizeBase64 rejects whitespace-only input", + actual: canonicalizeBase64(" \r\n\t"), + expected: undefined, + }, { name: "estimateBase64DecodedBytes handles whitespace", actual: estimateBase64DecodedBytes("SGV s bG8= \n"), diff --git a/packages/media-core/src/base64.ts b/packages/media-core/src/base64.ts index 828965254419..7926bac8ff7f 100644 --- a/packages/media-core/src/base64.ts +++ b/packages/media-core/src/base64.ts @@ -37,8 +37,6 @@ export function estimateBase64DecodedBytes(base64: string): number { return Math.max(0, estimated); } -const CANONICALIZE_BASE64_CHUNK_SIZE = 8192; - function isBase64DataChar(code: number): boolean { return ( (code >= 0x41 && code <= 0x5a) || @@ -67,25 +65,26 @@ function base64DataValue(code: number): number { * base64 only when the input has valid alphabet, padding, and length. */ export function canonicalizeBase64(base64: string): string | undefined { - const chunks: string[] = []; - let current = ""; - let cleanedLength = 0; + // Single validating pass; the output buffer is allocated lazily on the first + // whitespace and bounded by the input length, so canonical input returns + // unchanged with zero allocations and no input shape multiplies intermediates. + let out: Buffer | undefined; + let outLen = 0; let padding = 0; let sawPadding = false; let lastDataCode = 0; - const append = (char: string): void => { - current += char; - cleanedLength += 1; - if (current.length >= CANONICALIZE_BASE64_CHUNK_SIZE) { - chunks.push(current); - current = ""; - } - }; - for (let i = 0; i < base64.length; i += 1) { const code = base64.charCodeAt(i); if (code <= 0x20) { + if (out === undefined) { + // First whitespace: backfill the validated prefix [0, i). + out = Buffer.allocUnsafe(base64.length - 1); + for (let j = 0; j < i; j += 1) { + out[j] = base64.charCodeAt(j); + } + outLen = i; + } continue; } if (code === 0x3d) { @@ -94,32 +93,31 @@ export function canonicalizeBase64(base64: string): string | undefined { return undefined; } sawPadding = true; - append("="); - continue; - } - if (sawPadding || !isBase64DataChar(code)) { + } else if (sawPadding || !isBase64DataChar(code)) { return undefined; + } else { + lastDataCode = code; + } + if (out !== undefined) { + out[outLen] = code; + outLen += 1; } - lastDataCode = code; - append(base64[i] ?? ""); } + const cleanedLength = out === undefined ? base64.length : outLen; if (cleanedLength === 0) { return undefined; } const remainder = cleanedLength % 4; - if (remainder !== 0) { - if (sawPadding || remainder === 1) { - return undefined; - } - current += "=".repeat(4 - remainder); + if (remainder !== 0 && (sawPadding || remainder === 1)) { + return undefined; } const effectivePadding = remainder === 0 ? padding : 4 - remainder; const padBitMask = effectivePadding === 2 ? 0x0f : effectivePadding === 1 ? 0x03 : 0; if (padBitMask !== 0 && (base64DataValue(lastDataCode) & padBitMask) !== 0) { return undefined; } - if (current) { - chunks.push(current); - } - return chunks.join(""); + // Every kept character was validated against the base64 alphabet (ASCII), + // so a latin1 decode reproduces them exactly. + const cleaned = out === undefined ? base64 : out.subarray(0, outLen).toString("latin1"); + return remainder === 0 ? cleaned : cleaned + "=".repeat(4 - remainder); }