mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 20:05:46 -06:00
fix: large base64 attachments on /v1/responses crash the gateway with heap OOM (#126017)
* 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 <vincentkoc@ieee.org>
This commit is contained in:
@@ -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"),
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user