[robobun]

Problem

• A non-string encoding argument gets a different coercion than in Node. Buffer.byteLength("abcd", ["hex"]) returns 4 (Node: 2), Buffer.byteLength("abcd", Symbol()) returns 4 (Node: TypeError), and buf.toString({ valueOf: () => "hex" }) throws ERR_UNKNOWN_ENCODING (Node: hex output). buf.write and buf.indexOf behave like toString.
• Node's getEncodingOps does encoding += ''. That is ToPrimitive with the default hint, so valueOf() runs before toString() and a Symbol throws. Bun's parseEncoding(JSValue) (src/jsc/bindings/JSBuffer.cpp:334) and indexOf (:1844) used ToString (hint "string"). byteLength (:834) ignored every encoding that is not a string primitive.

Fix

• Add encodingToString: toPrimitive(NoPreference) then toString. parseEncoding(JSValue) and indexOf use it.
• Buffer.byteLength follows Node's order. A non-string input and an empty string return before the encoding is read. A falsy encoding is utf8. Anything else is coerced, and an unknown name falls back to utf8.
• Correct because every string, undefined, or null encoding produces the same result as before. Only object and Symbol encodings change, and they now match Node v26.3.0 on every case in the new test.
• Verified: test/js/node/buffer.test.js, describe "non-string encoding argument is coerced like Node's getEncodingOps" (4 tests, all fail on bun 1.4.3). The whole file passes except for tests that time out under the debug build when the full file runs and pass alone.

Background

• buf.fill and Buffer.alloc(size, fill, encoding) are unchanged. They coerce an object encoding through toString on purpose where Node throws ERR_INVALID_ARG_TYPE, and the fill tests pin that.
• Buffer.from(string, encoding) already matches Node and is unchanged.
• This composes with buffer: match Node's argument handling in Buffer.prototype.toString #43432 (toString argument order). That PR keeps its parseEncoding(scope, globalObject, arg1, false) call, which is the call site this helper feeds.
• node:buffer: an encoding that is not a string is coerced differently than in Node #43426 was filed from the bot account after a differential check against Node. No user has reported it.

Fixes #43426

Notes

Repro output after the fix, same order as the issue table: 2, 2, throws TypeError, "616263", "616263", 2, 0. Node v26.3.0 prints the same.

Extra cases checked against Node and pinned in the test: Buffer.byteLength("", Symbol()) is 0 and Buffer.byteLength(Buffer.alloc(3), Symbol()) is 3 (the encoding is never read), Buffer.byteLength("abcd", {}) is 4 (unknown name falls back to utf8), an object with Symbol.toPrimitive receives hint "default".

Self-reviewed: 4 concerns raised. 2 addressed in this body (provenance of the issue, relation to #43432). 2 were about priority, not correctness, and need no code change.

Full test/js/node/buffer.test.js run under the debug build: 678 pass, the failures are every fill() argument shape, reads match a DataView, BigInt writes match a DataView, and offset throws when a detaching valueOf runs mid-coercion. Each runs 6 to 10 s in the full file against a 5 s limit, the set changes between runs, and each passes when run alone. They pass on the release build.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/buffer.test.js

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 31571089-3f3d-4791-97c5-d2c848614ec0

📥 Commits

Reviewing files that changed from the base of the PR and between 2365aa1 and 687b1a3.

📒 Files selected for processing (1)

• src/jsc/bindings/JSBuffer.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

Walkthrough

Changes

Node buffer encoding coercion

Layer / File(s)	Summary
Encoding coercion implementation src/jsc/bindings/JSBuffer.cpp	Encoding objects now use ToPrimitive before string conversion. byteLength handles empty strings and UTF-8 fallbacks as specified. Buffer search uses the same conversion path.
Encoding coercion validation test/js/node/buffer.test.js	Tests cover coercion across byteLength, toString, write, and indexOf, including fallback behavior and conversion errors.

Suggested reviewers: dylan-conway

Priority: ➖ Normal

Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: matching Node.js encoding coercion for non-string buffer encodings.
Description check	✅ Passed	The description explains the problem, implementation, scope, compatibility goals, and verification results. It provides the information required by the template, although it uses Problem and Fix headi…
Linked Issues check	✅ Passed	Issue #43426 requires Node-compatible default-hint coercion for Buffer.byteLength, buf.toString, buf.write, and buf.indexOf. JSBuffer.cpp now uses ToPrimitive with no preference before str…
Out of Scope Changes check	✅ Passed	The changes address encoding coercion and related Buffer.byteLength and buffer search behavior. The tests directly cover issue #43426. The summary shows no changes to buf.fill, `Buffer.alloc(size,…

---

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

[claude]

Beyond the inline findings, I also checked the new encodingToString helper and the restructured byteLength for exception-scope hygiene: every toPrimitive/toString/view()/value() call is followed by RETURN_IF_EXCEPTION, toBoolean on the encoding cannot run user JS, and the string pointer is only read after the coercion that could run user code. I also looked at Bun's internal node:http/net/child_process callers of Buffer.byteLength — they pass string or undefined encodings, so the new object/Symbol coercion path is not reachable from them.

Extended reasoning...

Findings were reported inline, so this body only records what else was examined and ruled out. The C++ helper follows the ThrowScope conventions in the file (RETURN_IF_EXCEPTION after each throwing call, RELEASE_AND_RETURN on the tail call), toBoolean on a JSValue does not enter user code, and str is a JSString the EnsureStillAliveScope keeps rooted across the encoding coercion. The two ruled-out candidates from the hunt (internal callers passing object encodings, and existing callers relying on toString() on an encoding object) do not apply because Bun's built-in modules only pass string/undefined encodings and the new behavior matches Node for the object case. The inline findings (mustMatch third argument, write's validation order, and the indexOf test shape) remain the substantive concerns for a human to weigh.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/jsc/bindings/JSBuffer.cpp — Callers passing a non-string value and a Symbol encoding to buf.write get ERR_INVALID_ARG_TYPE in Bun where Node throws the Symbol-to-string TypeError. src/jsc/bindings/JSBuffer.cpp:2610 validates string before parseEncoding at :2619, while Node's write coerces the encoding first (getEncodingOps) and only validates the string inside ops.write. The diff adds the Symbol throw at :339-341 but keeps it behind the string check, so the doubly-invalid ordering now differs in error code as well as message. Fix: on the write paths (:2581 and :2619) coerce and validate the encoding before validateString at :2610, matching Node's ordering so the same error wins on doubly-invalid input.

  Extended reasoning...

  Node's Buffer.prototype.write: offset/length are validated, then if (!encoding) return this.utf8Write(...), then const ops = getEncodingOps(encoding) which does encoding += '' and throws TypeError "Cannot convert a Symbol value to a string" for a Symbol, and only then ops.write(this, string, ...) where the native binding rejects a non-string with ERR_INVALID_ARG_TYPE. So Buffer.alloc(4).write(123, 0, 4, Symbol()) in Node throws the Symbol TypeError with no code. In Bun :2610 runs Bun::V::validateString first and throws ERR_INVALID_ARG_TYPE for "string"; :2619 is never reached. Likewise buf.write(123, 0, 4, {}) throws ERR_INVALID_ARG_TYPE in Bun but ERR_UNKNOWN_ENCODING in Node. The base branch had the same order, but the PR's stated goal is to match Node's getEncodingOps contract on write, and the new tests at test/js/node/buffer.test.js:4271-4279 only cover a valid string, so the ordering gap is untested. Population: code that catches by error code. The dismissing finder read both but treated 'both TypeErrors' as equivalent; the codes differ.

  Verification: pre-existing (nit-level). Trigger: buf.write called with a non-string string argument together with a Symbol (or any throwing/unknown) encoding in the offset/length form, e.g. Buffer.alloc(4).write(123, 0, 4, Symbol()). Mechanism verified in /home/claude/bun/src/jsc/bindings/JSBuffer.cpp: line 2610 Bun::V::validateString(scope, lexicalGlobalObject, stringValue, "string"_s); runs first…

[robobun]

On the buf.write ordering finding: confirmed. Node v26.3.0 coerces the encoding before it validates string in the write(string, offset, length, encoding) form, so Buffer.alloc(4).write(123, 0, 4, Symbol()) throws the Symbol TypeError there and ERR_INVALID_ARG_TYPE here. That order predates this PR, and the write(string, encoding) form has a separate pre-existing gap (Bun does not validate string there at all, so write(123, "hex") returns 1 where Node throws). #30138 is open on the same lines. I am leaving both out of this PR to keep it to the encoding coercion.

[robobun]

^{Updated 7:08 AM PT - Sep 19th, 2026}

❌ @robobun, your commit 687b1a3 has 1 failures in Build #118306 (All Failures):

• test/js/bun/spawn/spawn.test.ts - code 1: stdout reader of an unref'd child and process lifetime > an idle reader stopped at the highwater … on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 43442

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

bun-43442 --bun

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

CI status: the only red lane in build 118306 is test/js/bun/spawn/spawn.test.ts on debian x64-asan, which also fails on main. The other failures are flaky tests that passed on retry or alone (bun-lock, watch-mode kill signal, fetch.tls.wildcard, fs.watch.rewrite). None touch node:buffer. The diff is ready for review.

[robobun]

A parallel branch has the same fix: robobun/56f69c63/buffer-encoding-coercion (25a163e). This PR owns the change, so no second PR is open. The C++ on that branch differs from this PR in one place. encodingToString returns a string primitive before it calls toPrimitive.

The branch has more tests (14, in test/js/node/buffer.test.js). They can move here if they are useful:

• lastIndexOf and includes, and indexOf with a Uint8Array needle.
• One coercion per call.
• An error thrown from valueOf(), and a null-prototype object.
• A Date subclass (the default hint of a Date means "string").
• Buffer.byteLength with an ArrayBuffer or a number as the first argument and an encoding that throws.

Two results from that branch apply here too, because the C++ is the same:

• A probe of 33 encoding values in 13 calls prints the same output on Node v26.3.0 and on that build. String and falsy encodings print the same output as canary 1.4.3.
• The probe also used an encoding whose valueOf() or Symbol.toPrimitive detaches or shrinks the buffer. The ASAN build reports nothing for toString, write, indexOf, lastIndexOf and includes.

The new describe in this PR sits at the same place as the new describe in #43368 (after "Buffer.fill offset/end argument handling"). The PR that merges second gets a conflict in buffer.test.js. The parallel branch puts its block before "read*/write* after JIT tier-up" for this reason.