Reject zero-length input in deserialize() instead of returning null

[robobun]

Problem

v8.deserialize() (and bun:jsc's deserialize()) manufactures a null out of zero bytes instead of throwing:

const v8 = require("node:v8");
for (const buf of [Buffer.alloc(0), new Uint8Array(0), new DataView(new ArrayBuffer(0))]) {
  try { console.log("->", v8.deserialize(buf)); } catch (e) { console.log("threw", e.name); }
}
// node: all three -> threw Error
// bun:  all three -> null

An empty buffer is what a truncated file, an empty IPC frame, or a zero-length redis/S3 value looks like, which is exactly what the wire-format header check exists to catch. Code that stores serialize() output and later deserialize()s it gets back an ordinary-looking application value instead of an error.

Every other malformed payload already throws, so zero-length was the only hole:

input	before
Buffer.alloc(0)	null
Buffer.from([0])	TypeError: Unable to deserialize data.
Buffer.from("abc")	TypeError: Unable to deserialize data.
v8.serialize({a:1}).subarray(0, 2)	TypeError: Unable to deserialize data.

Cause

CloneDeserializer::deserialize maps an empty buffer to (jsNull(), SerializationReturnCode::UnspecifiedError). That is WebCore's null-value sentinel: an empty m_data is how SerializedScriptValue::nullValue() is represented, so maybeThrowExceptionIfSerializationFailed deliberately leaves UnspecifiedError unthrown.

SerializedScriptValue::fromArrayBuffer is the entry point for bytes supplied by a caller (its only caller is functionDeserialize in BunJSCModule.h, behind bun:jsc's deserialize(), which node:v8's deserialize() wraps). It passed the sentinel straight through and returned the jsNull().

Fix

fromArrayBuffer now rejects an empty span before handing it to the deserializer. Bytes from a caller carry no sentinel meaning: zero bytes have no version header, so they fail with the same ValidationError every other unparsable payload already produces (TypeError: Unable to deserialize data.).

This covers Buffer, Uint8Array, DataView, ArrayBuffer, SharedArrayBuffer, and a zero-length view over a non-empty backing buffer. structuredClone(), postMessage(), and child_process IPC go through SerializedScriptValue::deserialize, a different function, and are untouched.

The thrown error's class and message still differ from Node's (TypeError: Unable to deserialize data. vs Error: Unable to deserialize cloned data due to invalid or unsupported version.). That divergence already applies to every malformed payload, because Bun's wire format is JSC's structured clone rather than V8's; matching Node only for the empty case would have made v8.deserialize inconsistent with itself.

Verification

New tests in test/js/bun/jsc/bun-jsc.test.ts cover both APIs, all six empty shapes, and the two things that must keep working: a value that really is null, and a serialized empty Buffer.

$ USE_SYSTEM_BUN=1 bun test test/js/bun/jsc/bun-jsc.test.ts -t "deserialize rejects input with no bytes"
 2 pass
 6 fail

$ bun bd test test/js/bun/jsc/bun-jsc.test.ts -t "deserialize rejects input with no bytes"
 8 pass
 0 fail

Also green: the whole bun-jsc.test.ts file, test/js/web/structured-clone-blob-file.test.ts, test/js/web/workers/structured-clone.test.ts, structuredClone-classes.test.ts, worker-postmessage-transfer.test.ts, and Node's test/parallel/test-v8-deserialize-buffer.js. BUN_JSC_validateExceptionChecks=1 reports no unchecked exception on the new throw path.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 21 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ff9b0379-3661-453f-9fbe-e7dcc4049f85

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and 637e741.

📒 Files selected for processing (2)

• src/jsc/bindings/webcore/SerializedScriptValue.cpp
• test/js/bun/jsc/bun-jsc.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 4:05 PM PT - Jul 6th, 2026}

❌ @robobun, your commit 637e741 has some failures in Build #69176 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 33465

That installs a local version of the PR into your bun-33465 executable, so you can run:

bun-33465 --bun

[claude]

LGTM — narrow guard mirroring the detached-buffer check right above it, with thorough tests.

Extended reasoning...

Overview

Adds a zero-length guard to SerializedScriptValue::fromArrayBuffer in src/jsc/bindings/webcore/SerializedScriptValue.cpp so that bun:jsc deserialize() and v8.deserialize() throw on empty input instead of leaking WebCore's internal null-value sentinel. The size computation is hoisted a few lines up (pure move, no semantic change) and a new if (!size) block is inserted that mirrors the existing !arrayBuffer || arrayBuffer->isDetached() guard immediately above it: sets *didFail, conditionally throws via maybeThrowExceptionIfSerializationFailed(..., ValidationError), and returns jsUndefined(). Tests in test/js/bun/jsc/bun-jsc.test.ts cover six empty-buffer shapes across both APIs plus round-trip guards for null and an empty Buffer.

Security risks

None. This tightens input validation (rejects malformed input that was previously silently accepted as null). No new parsing, no new allocation, no untrusted data flows introduced.

Level of scrutiny

Low-to-moderate. The change is ~15 lines of C++ in a Bun-specific entry point — fromArrayBuffer's only caller is functionDeserialize in BunJSCModule.h; structuredClone, postMessage, and IPC go through a different function and are untouched. The new block is a copy of the pattern directly above it, uses the existing ValidationError → TypeError: Unable to deserialize data. path (verified at SerializedScriptValue.cpp:5966-5967), and has correct exception-scope handling (RETURN_IF_EXCEPTION after the maybe-throw). It is a user-facing behavior change, but one that aligns Bun with Node.js and closes the only remaining hole in an otherwise consistent "malformed payload throws" contract.

Other factors

No bugs from the bug hunter. No CODEOWNERS on this path. Tests are comprehensive (parameterized over Buffer/Uint8Array/DataView/ArrayBuffer/SharedArrayBuffer/zero-length-view-of-nonempty-buffer) and include the two obvious regression guards. PR description documents USE_SYSTEM_BUN=1 fail / bun bd pass and BUN_JSC_validateExceptionChecks=1 clean. No prior reviewer comments to address.

[claude]

LGTM — small, well-scoped validation fix that mirrors the existing detached-buffer guard, with thorough test coverage.

Extended reasoning...

Overview

This PR closes a hole in bun:jsc / node:v8 deserialize(): zero-length input previously returned null (via WebCore's null-value sentinel) instead of throwing like every other malformed payload. The fix adds an early if (!size) guard in SerializedScriptValue::fromArrayBuffer (SerializedScriptValue.cpp:6646-6660) that emits ValidationError through the existing maybeThrowExceptionIfSerializationFailed path, and adds a matching test block in test/js/bun/jsc/bun-jsc.test.ts.

Security risks

None. The change tightens input validation on user-supplied bytes rather than loosening it. No new parsing, allocation, or external I/O is introduced.

Level of scrutiny

Low-to-moderate. The new guard is a near-verbatim copy of the detached-ArrayBuffer guard immediately above it (same didFail write, same throwExceptions gate, same jsUndefined() return), and reuses the existing ValidationError → TypeError: Unable to deserialize data. mapping so the error is consistent with other malformed inputs. RETURN_IF_EXCEPTION is present after the throw. The size computation was hoisted a few lines earlier but is a pure read of arrayBuffer->byteLength() with no ordering dependency on the code it moved past. I confirmed fromArrayBuffer's only C++ caller is functionDeserialize in BunJSCModule.h, so structuredClone/postMessage/IPC (which go through SerializedScriptValue::deserialize) are unaffected — matching the PR description.

Other factors

Test coverage is strong: 6 empty-input shapes (including a zero-length view over a non-empty backing buffer) exercised through both bun:jsc and node:v8, plus positive guards that serialize(null) and serialize(Buffer.alloc(0)) still round-trip. The description documents that the new tests fail under USE_SYSTEM_BUN=1 and pass under bun bd, and that BUN_JSC_validateExceptionChecks=1 is clean. The bug-hunting system found no issues, and there are no prior reviewer comments on the PR.

[robobun]

CI triage — diff is green, remaining red is infra

The diff is unchanged throughout: 18 lines in SerializedScriptValue.cpp, 32 in bun-jsc.test.ts, compiling on every build-cpp lane that got an agent (linux x64 / aarch64 / baseline / asan / musl / android, freebsd x64 + aarch64, windows x64). No failure in any run has touched structured-clone / deserialize code.

08dfbef7b0 (#68877): stale base, not flake. cookie-map.test.ts had three broken Expires assertions at my branch point, fixed by the next commit on main (#33425). Rebasing cleared them.

8f367de5c3 (#68921): 270 passed / 3 failed / 13 expired.

• darwin 26 aarch64 (x2): buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun' — the test binary never downloaded, no test ran.
• darwin 14 x64: terminal.test.ts > Bun.spawn with terminal option timed out after 90s; that file has zero references to serialize/deserialize/bun:jsc and the same test flaked on #68923 (unrelated branch).
• 13 expired: Buildkite never assigned the jobs (pipeline-wide at the time: #68908 expired 41, #68920 27, #68900 27).

637e741a77 (#69176, one-time retrigger): 281 passed / 1 failed. The single failure is the same darwin-aarch64-build-bun artifact-download timeout as above — again, no test ran. The one other red (update_interactive_install.test.ts) retried and passed (flaky).

The recurring darwin-aarch64 artifact-download timeout is CI infrastructure (S3 / agent), reproducing identically across two independent builds and unrelated to this change. I've used my one retrigger, so I'm leaving it here rather than pushing more empty commits. The fix itself is green and reviewed; this needs a maintainer to merge or to kick the darwin artifact step.

[claude]

LGTM — focused Node compat fix that mirrors the existing detached-buffer guard, with thorough test coverage.

Extended reasoning...

Overview

Adds a zero-length guard to SerializedScriptValue::fromArrayBuffer in src/jsc/bindings/webcore/SerializedScriptValue.cpp so that bun:jsc deserialize() and v8.deserialize() throw on empty input instead of returning null (WebCore's null-value sentinel leaking through). Tests added to test/js/bun/jsc/bun-jsc.test.ts cover six empty-input shapes across both APIs plus regression guards for serialized null and serialized empty Buffer.

Security risks

None. This tightens validation of caller-supplied bytes (fail-closed rather than fail-open). No new parsing, no untrusted-size arithmetic, no allocation.

Level of scrutiny

Low-to-moderate. The C++ change is ~15 lines that copy the exact shape of the detached-buffer guard immediately above it: set *didFail, gate on throwExceptions == Throwing, call maybeThrowExceptionIfSerializationFailed(..., ValidationError) (which throws the same TypeError: Unable to deserialize data. as every other malformed payload), RETURN_IF_EXCEPTION, return jsUndefined(). The size = std::min(...) computation was hoisted above the new guard unchanged. I confirmed fromArrayBuffer's only callers are the two sites in BunJSCModule.h::functionDeserialize, so structuredClone/postMessage/IPC are untouched as claimed.

Other factors

• The PR description includes root-cause analysis, a before/after table, USE_SYSTEM_BUN=1 failure verification, BUN_JSC_validateExceptionChecks=1 validation, and green runs on adjacent structured-clone/worker test files.
• CI triage shows 107 passed / 0 failed (only agent-starvation expirations, pipeline-wide).
• No CODEOWNERS on the touched paths.
• No prior human review comments to address.