Skip to content

fix nested buffer containing array of numbers instead of base64 #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tobiasvdorp wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix nested buffer containing array of numbers instead of base64 #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/cache-handler/src/helpers/serialization.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,32 @@ describe("serialization helpers", () => {
});
});

describe("nested Buffer (Node toJSON before replacer)", () => {
test("serializes rscData as base64, not int-array JSON", () => {
const entry = { rscData: Buffer.from('1:"test"') };
const serialized = JSON.stringify(entry, jsonReplacer);

expect(serialized).toContain('"__serialized_type":"Buffer"');
expect(serialized).toContain('"data":"MToidGVzdCI="');
expect(serialized).not.toContain('"type":"Buffer"');
expect(serialized).not.toMatch(/"data":\[\d+,/);
});

test("round-trips rscData nested in an APP_PAGE-like object", () => {
const entry = {
kind: "APP_PAGE",
rscData: Buffer.from("page-rsc-payload"),
html: "<html></html>",
};
const result = roundTrip(entry) as typeof entry;

expect(Buffer.isBuffer(result.rscData)).toBe(true);
expect(result.rscData.toString()).toBe("page-rsc-payload");
expect(result.kind).toBe("APP_PAGE");
expect(result.html).toBe("<html></html>");
});
});

describe("Next.js APP_PAGE shape (Map<string, Buffer>)", () => {
test("round-trips a Map of Buffers (segmentData)", () => {
const segmentData = new Map<string, Buffer>([
Expand Down Expand Up @@ -107,6 +133,13 @@ describe("serialization helpers", () => {
expect(result).toEqual(userValue);
});

test("does not convert { type: 'Buffer' } with invalid byte values", () => {
const userValue = { type: "Buffer", data: [1, 256, 3.5] };
const result = roundTrip(userValue);
expect(Buffer.isBuffer(result)).toBe(false);
expect(result).toEqual(userValue);
});

test("does not convert a fake Map marker with malformed entries", () => {
const userValue = { __serialized_type: "Map", entries: "not-an-array" };
const result = roundTrip(userValue);
Expand Down
32 changes: 26 additions & 6 deletions packages/cache-handler/src/helpers/serialization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,22 @@
* loses Buffer identity, so custom replacer/reviver functions are needed.
*/

type NodeBufferJson = {
type: "Buffer";
data: number[];
};

function isByte(value: unknown): value is number {
return typeof value === "number" && Number.isInteger(value) && value >= 0 && value <= 255;
}

function isNodeBufferJson(value: unknown): value is NodeBufferJson {
if (!value || typeof value !== "object") return false;

const obj = value as Record<string, unknown>;
return obj.type === "Buffer" && Array.isArray(obj.data) && obj.data.every(isByte);
}
Comment on lines +18 to +23

@gemini-code-assist gemini-code-assist Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using Array.prototype.every to check every single element of a potentially large nested Buffer (which can be several megabytes in size) is a major performance bottleneck. This check runs during both serialization and deserialization on every cache read/write, which can block the event loop and degrade request latency.

To optimize this, we can perform a full scan only for small arrays (e.g., length <= 100) and use a deterministic sampling check (e.g., checking the first, middle, and last elements) for larger arrays. This maintains high accuracy for typical false-positive cases while keeping the check $O(1)$ for large payloads.

function isNodeBufferJson(value: unknown): value is NodeBufferJson {
  if (!value || typeof value !== "object") return false;

  const obj = value as Record<string, unknown>;
  if (obj.type !== "Buffer" || !Array.isArray(obj.data)) return false;

  const len = obj.data.length;
  if (len === 0) return true;

  if (len <= 100) {
    return obj.data.every(isByte);
  }

  return (
    isByte(obj.data[0]) &&
    isByte(obj.data[len - 1]) &&
    isByte(obj.data[Math.floor(len / 2)]) &&
    isByte(obj.data[Math.floor(len / 4)]) &&
    isByte(obj.data[Math.floor((3 * len) / 4)])
  );
}

@tobiasvdorp tobiasvdorp Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gemini-code-assist I checked this against the previous behavior and did a small benchmark.
A full array scan already existed on main in jsonReviver for old/native Buffer JSON entries:

obj.type === "Buffer" &&
Array.isArray(obj.data) &&
obj.data.every((n) => typeof n === "number")

This PR adds the same detection on the write path because nested Buffers are converted by Node’s Buffer.toJSON() before the replacer sees them. New-format reads do not scan the large array anymore because they read { __serialized_type: "Buffer", data: "" }.

I also benchmarked RSC-like payloads. The current fix adds some write CPU, but substantially reduces stored size:

rscData size Write CPU Serialized size
64 KB 0.827ms -> 0.880ms 225KB -> 88KB
512 KB 6.38ms -> 6.74ms 1.80MB -> 700KB
1 MB 12.5ms -> 14.0ms 3.60MB -> 1.40MB
5 MB 65.9ms -> 79.6ms 18.0MB -> 7.0MB

So this is a write-side CPU tradeoff for about a 61% payload-size reduction in the tested RSC-like cases.

I’d prefer not to use deterministic sampling here because it weakens the serialization boundary: a Buffer-shaped object with invalid unsampled values could be silently converted.


/**
* Custom JSON replacer that serializes Map and Buffer instances.
* - Maps become `{ __serialized_type: "Map", entries: [...] }`
Expand All @@ -24,6 +40,14 @@ export function jsonReplacer(_key: string, value: unknown): unknown {
data: value.toString("base64"),
};
}
// Node calls Buffer.toJSON() before the replacer — nested Buffers arrive as
// `{ type: "Buffer", data: number[] }` instead of a Buffer instance.
if (isNodeBufferJson(value)) {
return {
__serialized_type: "Buffer",
data: Buffer.from(value.data).toString("base64"),
};
}
return value;
}

Expand Down Expand Up @@ -51,12 +75,8 @@ export function jsonReviver(_key: string, value: unknown): unknown {
// Backward compat: Node's Buffer.toJSON() format.
// Guard with number[] check to avoid false-positives on user data
// that happens to have { type: "Buffer", data: [...] } shape.
if (
obj.type === "Buffer" &&
Array.isArray(obj.data) &&
obj.data.every((n) => typeof n === "number")
) {
return Buffer.from(obj.data as number[]);
if (isNodeBufferJson(obj)) {
return Buffer.from(obj.data);
}
}
return value;
Expand Down