node:buffer: treat a detached view as empty in compare, equals, swapNN and as a fill value

[robobun]

Problem

• A Buffer (or Uint8Array) whose ArrayBuffer has been detached is rejected with a bare TypeError by Buffer.compare() (Uint8Array (first argument) is detached), buf.compare() and buf.equals() (Uint8Array is detached), buf.swap16()/swap32()/swap64() (Buffer is detached), and as the fill value of Buffer.alloc(n, view) / buf.fill(view) (Uint8Array is detached).
• Node has no detached special case in any of these. A detached view has length 0 there, so it compares as empty (Buffer.compare(detached, detached) is 0, Buffer.from("abc").compare(detached) is 1), equals() is true only against another empty view, swapNN() returns the buffer unchanged, and a detached fill value gets the same ERR_INVALID_ARG_VALUE an empty one gets.
• Cause: explicit isDetached() guards in src/jsc/bindings/JSBuffer.cpp (on main at lines 718, 843, 853, 1115, 1324, 1453, 1965, 1994 and 2028). Pre-existing, reproduces on 1.3.14 and the current canary; not a regression.

Fix

• Removes those nine guards. Nothing else changes.
• Correct because JSC, like V8, gives a detached view byteLength() 0 (JSArrayBufferView::detachFromArrayBuffer sets the length to 0 and clears the data pointer), and each body already handles a zero-length view exactly as Node does: both compare bodies and equals skip the memcmp when the shorter length is 0 and order on lengths alone, the swapNN loops run zero iterations and return this, and alloc/fill hit their existing length == 0 branch, which throws ERR_INVALID_ARG_VALUE. The guards were the only source of the divergence.
• The detached view's null data pointer is never dereferenced: every memory access in these bodies is behind a non-zero length, and no user JS can run between the length read and the access (compare's offsets go through validateInteger, which rejects anything that is not already a number; fill reads the value's length after all argument coercion). A length-tracking view on a resizable buffer takes JSC's other byteLength() path and also reads 0 once detached; the tests cover it.
• Verified with the new describe("detached buffers in compare, equals, swapNN and as a fill value") block in test/js/node/buffer.test.js: all 8 tests fail on the released build with the TypeErrors above and pass with this change on a debug (ASAN + UBSan) build. Expected values were taken from Node v26.3.0 running the same calls.
• Also passing: the rest of buffer.test.js (640 tests), buffer-copy-fill-detach.test.ts, buffer-indexOf-detach.test.ts, and upstream test-buffer-compare.js, test-buffer-compare-offset.js, test-buffer-equals.js, test-buffer-swap.js, test-buffer-fill.js, test-buffer-alloc.js.
• Not changed: the isDetached() checks that remain in the file are places where Node throws too (Buffer.concat, slice/subarray, Buffer.from(detachedArrayBuffer)), the indexOf check that returns -1, and the write() re-checks that Buffer.write: Node-compatible errors when toString() detaches or shrinks the buffer #30138 replaces. detached.fill(1) returning the buffer where Node throws, and the wording of ERR_OUT_OF_RANGE / fill's ERR_INVALID_ARG_VALUE messages, are pre-existing and unrelated to the detached guards.

Background

• Detaching: ArrayBuffer.prototype.transfer() and structuredClone(ab, { transfer }) move an ArrayBuffer's memory out from under its views. The engine then reports every view on it as length 0 with no data pointer; the views themselves stay usable objects.
• In Node, Buffer.compare, buf.compare, buf.equals and swapNN are JS wrappers over this.length/byteLength plus a native compare that uses V8's view length, and fill passes the value view's length to native code. None of them look at the detached state, which is why a detached view simply behaves as an empty one there.
• The Bun functions here are C++ bindings over the same JSC view object (JSUint8Array). byteLength() is the length JSC reports (0 once detached); vector()/typedVector() is the data pointer (null once detached).

Node v26.3.0 vs Bun, before and after (`det` = 16-byte Buffer whose ArrayBuffer was transferred, `abc` = Buffer.from("abc"), `empty` = Buffer.alloc(0))

                                node                      bun before                                        bun after
Buffer.compare(det, det)        0                         TypeError: Uint8Array (first argument) is detached   0
Buffer.compare(det, abc)        -1                        TypeError (first argument)                        -1
Buffer.compare(abc, det)        1                         TypeError (second argument)                       1
Buffer.compare(empty, det)      0                         TypeError (second argument)                       0
abc.compare(det)                1                         TypeError: Uint8Array is detached                 1
empty.compare(det)              0                         TypeError                                         0
abc.compare(det, 5)             1                         TypeError                                         1
abc.compare(det, 0, 1)          ERR_OUT_OF_RANGE          TypeError                                         ERR_OUT_OF_RANGE
det.compare(abc)                -1                        -1 (no guard on this side)                        -1
abc.equals(det)                 false                     TypeError: Uint8Array is detached                 false
empty.equals(det)               true                      TypeError                                         true
det.equals(det)                 true                      TypeError                                         true
det.equals(abc)                 false                     false (no guard on this side)                     false
det.swap16/32/64()              returns det (length 0)    TypeError: Buffer is detached                     returns det
det(3 bytes).swap16()           returns det               TypeError                                         returns det
Buffer.alloc(4, det)            ERR_INVALID_ARG_VALUE     TypeError: Uint8Array is detached                 ERR_INVALID_ARG_VALUE
abc.fill(det)                   ERR_INVALID_ARG_VALUE     TypeError: Uint8Array is detached                 ERR_INVALID_ARG_VALUE
Buffer.alloc(0, det)            empty Buffer              empty Buffer                                      empty Buffer
abc.fill(det, 1, 1)             returns abc               returns abc                                       returns abc

Unchanged, and the same kind of error in Node: Buffer.concat([det]), det.slice(), det.subarray() and Buffer.from(detachedArrayBuffer) all throw a TypeError in both.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 4 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

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: e4bd2384-e944-4163-8676-d424c700e2ef

📥 Commits

Reviewing files that changed from the base of the PR and between 771c7e6 and e7bd77d.

📒 Files selected for processing (2)

• src/jsc/bindings/JSBuffer.cpp
• test/js/node/buffer.test.js

---

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

[robobun]

^{Updated 9:42 PM PT - Aug 16th, 2026}

✅ @robobun, your commit e7bd77d59348c8387f2469f9554c84234ca57374 passed in Build #99846! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39406

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

bun-39406 --bun

[robobun]

Status: reproduced and fixed in this PR.

• Reproduced on the released build (USE_SYSTEM_BUN=1 bun test test/js/node/buffer.test.js -t "detached buffers in compare"): all 8 new tests fail with TypeError: Uint8Array (first argument) is detached / Uint8Array is detached / Buffer is detached; Node v26.3.0 returns the values the tests assert.
• With this change (bun bd test test/js/node/buffer.test.js) the new block and the rest of the file pass, along with the upstream test-buffer-{compare,compare-offset,equals,swap,fill,alloc}.js.
• Running a self-review of the diff next; will update here if anything comes out of it.

[claude]

LGTM — the removed isDetached() guards were the only divergence from Node, and each body already handles the resulting zero-length view without touching the null vector.

What was reviewed:

• Traced each of the nine removed guards: memcmp in compare/equals is gated on actualLength > 0; buf.compare's early-return branches (sourceStart >= sourceEnd / targetStart >= targetEnd) fire before typedVector() is read; swapNN loops run zero iterations; alloc/fill throw ERR_INVALID_ARG_VALUE on length == 0 before vector().
• Confirmed no TOCTOU window in buf.compare — validateInteger rejects non-numbers up front (NodeValidator.cpp:69) and its min/max are jsNumber literals, so no user coercion runs between the byteLength() read and the memcmp.
• Checked that nullptr + 0 in the constructor compare path is well-defined; length-tracking resizable views are covered by the tests.

Extended reasoning...

Overview

Removes nine explicit isDetached() guards from src/jsc/bindings/JSBuffer.cpp in Buffer.compare, buf.compare, buf.equals, buf.swap16/32/64, and the view-fill branch of Buffer.alloc/buf.fill. Adds an 8-test describe block to test/js/node/buffer.test.js asserting Node v26's behavior for detached views in each of these paths, including length-tracking views on resizable ArrayBuffers.

Security risks

The concern with removing detached guards is null-pointer dereference (JSC's vector()/typedVector() return null once detached). I traced every removed guard to its downstream memory access: constructor compare and equals gate memcmp on actualLength > 0 / compare_length > 0; prototype compare returns from sourceStart >= sourceEnd or targetStart >= targetEnd before reaching typedVector(); the three swapNN bodies compute count = 0 and skip their loops; alloc and fill throw on length == 0 before vector() is read. No path dereferences null.

The other risk is a TOCTOU where user code detaches between the byteLength() read and the memory access. Verified in NodeValidator.cpp:48-66 that validateInteger rejects non-numbers before any coercion and its min/max args here are jsNumber(...) literals — no user JS runs in buf.compare's offset handling. fill reads viewValue->byteLength() after all argument coercion and no further side effects run before the write.

Level of scrutiny

Medium-high: this is native memory-touching code where removing a guard could produce a UAF/null-deref. However, the change is purely subtractive (deletes guards, adds comments), the safety argument is local to each function body and easy to verify by inspection, and the PR description states it passes on an ASAN+UBSan debug build. I read each function body end-to-end to confirm the null vector is never reached.

Other factors

Test coverage is thorough: both argument positions in compare/equals, explicit offsets triggering ERR_OUT_OF_RANGE against a detached side's length of 0, the detached(3).swap16() case proving the size check sees 0 not 3, length-tracking resizable views, and the empty-range fill no-op. Expected values are stated to be taken from Node v26.3.0. The PR also enumerates which isDetached() checks in the file are intentionally left alone (concat, slice/subarray, from(detachedArrayBuffer), indexOf, write) and why. No prior review comments; no CODEOWNERS on this path.