From 2862b4e2022905b3119942ce0f6c0db8008a6a21 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:01:11 +0000 Subject: [PATCH 1/8] buffer: match Node's Buffer.prototype.fill offset and end argument handling Buffer.prototype.fill reinterprets a string `offset` or `end` argument as the encoding regardless of the fill value's type, and always validates `end`. Node's _fill (lib/buffer.js) only performs that reinterpretation when the fill value is itself a string, and only reads `end` once `offset` is present. Eight argument shapes diverge: buf.fill(0, "1", 3) // bun fills [0,3); node ERR_INVALID_ARG_TYPE buf.fill(0, 1, "3") // bun fills [1,len); node ERR_INVALID_ARG_TYPE buf.fill(0, undefined, "3") // bun ERR_INVALID_ARG_TYPE; node fills all buf.fill(0, undefined, 3) // bun fills [0,3); node fills all buf.fill("b", "utf8", 3) // bun fills [0,3); node fills all Gate the string-offset/end to encoding reinterpretation on value.isString(), discard `end` in the offset-slot branch to match Node's `end = buf.length` reset, and nest the `end` validation inside the offset-present branch so it is never read without an `offset`. --- src/jsc/bindings/JSBuffer.cpp | 28 ++++++++----- test/js/node/buffer.test.js | 79 +++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 55cda93f9dd6..c426c3e078cb 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -1353,12 +1353,18 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec break; } - if (offsetValue.isUndefined() || offsetValue.isString()) { - encodingValue = offsetValue; - offsetValue = jsUndefined(); - } else if (endValue.isString()) { - encodingValue = endValue; - endValue = jsUndefined(); + // Node's _fill only reinterprets a string offset/end as the encoding when the + // fill value is itself a string; otherwise validateInteger below rejects them. + // The offset-slot branch also discards end: fill("a", "hex", 3) fills everything. + if (value.isString()) { + if (offsetValue.isUndefined() || offsetValue.isString()) { + encodingValue = offsetValue; + offsetValue = jsUndefined(); + endValue = jsUndefined(); + } else if (endValue.isString()) { + encodingValue = endValue; + endValue = jsUndefined(); + } } // ── 1. Encoding parse (FIRST validation) ──────────────────────────── @@ -1386,10 +1392,12 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec if (!offsetValue.isUndefined()) { Bun::V::validateInteger(scope, lexicalGlobalObject, offsetValue, "offset"_s, jsNumber(0), jsNumber(Bun::Buffer::kMaxLength), &offset); RETURN_IF_EXCEPTION(scope, {}); - } - if (!endValue.isUndefined()) { - Bun::V::validateInteger(scope, lexicalGlobalObject, endValue, "end"_s, jsNumber(0), jsNumber(limit), &end); - RETURN_IF_EXCEPTION(scope, {}); + // Node only reads `end` once `offset` is present: fill(v, undefined, end) + // ignores end (it is never validated) and fills the whole buffer. + if (!endValue.isUndefined()) { + Bun::V::validateInteger(scope, lexicalGlobalObject, endValue, "end"_s, jsNumber(0), jsNumber(limit), &end); + RETURN_IF_EXCEPTION(scope, {}); + } } // Node short-circuits empty/inverted ranges before coercing `value`, diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 4720caac09f6..0648d8d02b5c 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -3768,6 +3768,85 @@ describe("ERR_BUFFER_OUT_OF_BOUNDS", () => { } }); +// Node's _fill (lib/buffer.js) only reinterprets a string `offset`/`end` as +// the encoding when the fill value is itself a string; otherwise they reach +// validateOffset and a non-number throws ERR_INVALID_ARG_TYPE. And `end` is +// only read at all once `offset` is present. +describe("Buffer.fill offset/end argument handling", () => { + it("rejects a string offset when the fill value is not a string", () => { + for (const value of [0, true, new Uint8Array([1])]) { + expect(() => Buffer.alloc(5).fill(value, "1", 3)).toThrow( + expect.objectContaining({ + name: "TypeError", + code: "ERR_INVALID_ARG_TYPE", + message: `The "offset" argument must be of type number. Received type string ('1')`, + }), + ); + // Two-argument form: a string in the offset slot is not an encoding either. + expect(() => Buffer.alloc(5).fill(value, "hex")).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: `The "offset" argument must be of type number. Received type string ('hex')`, + }), + ); + } + expect(() => Buffer.alloc(5).fill(0, null, 3)).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: 'The "offset" argument must be of type number. Received null', + }), + ); + }); + + it("rejects a string end when the fill value is not a string", () => { + for (const value of [0, true, new Uint8Array([1])]) { + expect(() => Buffer.alloc(5).fill(value, 1, "3")).toThrow( + expect.objectContaining({ + name: "TypeError", + code: "ERR_INVALID_ARG_TYPE", + message: `The "end" argument must be of type number. Received type string ('3')`, + }), + ); + } + expect(() => Buffer.alloc(5).fill(0, 1, null)).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: 'The "end" argument must be of type number. Received null', + }), + ); + }); + + it("ignores end entirely when offset is undefined", () => { + // Node resets end = buf.length when offset === undefined; end is never + // validated, so even an otherwise invalid value there is accepted. + expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, undefined, 3))).toEqual([0, 0, 0, 0, 0]); + expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, undefined, "3"))).toEqual([0, 0, 0, 0, 0]); + expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, undefined, -1))).toEqual([0, 0, 0, 0, 0]); + expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, undefined, null))).toEqual([0, 0, 0, 0, 0]); + }); + + it("discards end when a string value's offset slot holds the encoding", () => { + // fill(value, encoding) has no end slot, so anything there is ignored. + expect(Buffer.alloc(5, 0xaa).fill("b", "utf8", 3).toString()).toBe("bbbbb"); + expect(Buffer.alloc(5, 0xaa).fill("b", undefined, 3).toString()).toBe("bbbbb"); + // An explicit numeric offset keeps end meaningful. + expect(Array.from(Buffer.alloc(5, 0xaa).fill("b", 1, 3))).toEqual([0xaa, 0x62, 0x62, 0xaa, 0xaa]); + }); + + it("still treats a string offset/end as the encoding when the value is a string", () => { + expect(Buffer.alloc(4, 0xaa).fill("ab", "utf16le").toString("hex")).toBe("61006200"); + expect(Buffer.alloc(4, 0xaa).fill("ab", 0, "utf16le").toString("hex")).toBe("61006200"); + expect(() => Buffer.alloc(4).fill("a", "bogus")).toThrow(expect.objectContaining({ code: "ERR_UNKNOWN_ENCODING" })); + expect(() => Buffer.alloc(4).fill("a", 0, "bogus")).toThrow( + expect.objectContaining({ code: "ERR_UNKNOWN_ENCODING" }), + ); + }); + + it("never treats the 4-argument encoding as meaningful for a non-string value", () => { + expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, 1, 3, "bogus"))).toEqual([0xaa, 0, 0, 0xaa, 0xaa]); + }); +}); + describe("*Write methods with NaN/invalid offset and length", () => { // Regression test: NaN offset/length values must be handled safely. // NaN offset should be treated as 0, and length should be clamped to buffer size. From 274affba1c8396f9f37043720a2a43075d2741c9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:18:34 +0000 Subject: [PATCH 2/8] test: diff every Buffer.fill argument shape between Node.js and Bun The new fixture enumerates 34 Buffer.prototype.fill argument shapes and prints the resulting bytes or the thrown error class and code. buffer.test.js spawns it under nodeExe() and bunExe() and requires the two outputs to be byte-identical. Node 24.3.0 (the version the CI image pins) and 26.3.0 agree on every line. --- test/js/node/buffer-fill-args-fixture.js | 66 ++++++++++++++++++++++++ test/js/node/buffer.test.js | 20 ++++++- 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/js/node/buffer-fill-args-fixture.js diff --git a/test/js/node/buffer-fill-args-fixture.js b/test/js/node/buffer-fill-args-fixture.js new file mode 100644 index 000000000000..f98597791371 --- /dev/null +++ b/test/js/node/buffer-fill-args-fixture.js @@ -0,0 +1,66 @@ +// Prints one deterministic line per Buffer.prototype.fill() argument shape. +// buffer.test.js runs this fixture under both Node.js and Bun and requires +// the two outputs to be byte-identical. +// https://github.com/oven-sh/bun/pull/33033 +"use strict"; + +const cases = [ + // value is NOT a string: Node never reinterprets a string offset/end as the encoding. + ['fill(0, "1", 3)', b => b.fill(0, "1", 3)], + ['fill(0, 1, "3")', b => b.fill(0, 1, "3")], + ['fill(0, "hex")', b => b.fill(0, "hex")], + ['fill(0, "bogus")', b => b.fill(0, "bogus")], + ['fill(true, "1", 3)', b => b.fill(true, "1", 3)], + ['fill(uint8, "1", 3)', b => b.fill(new Uint8Array([1]), "1", 3)], + ['fill(uint8, 1, "3")', b => b.fill(new Uint8Array([1]), 1, "3")], + ["fill(0, null, 3)", b => b.fill(0, null, 3)], + ["fill(0, 1, null)", b => b.fill(0, 1, null)], + + // `end` is never read (or validated) when `offset` is undefined. + ['fill(0, undefined, "3")', b => b.fill(0, undefined, "3")], + ["fill(0, undefined, 3)", b => b.fill(0, undefined, 3)], + ["fill(0, undefined, null)", b => b.fill(0, undefined, null)], + ["fill(0, undefined, -1)", b => b.fill(0, undefined, -1)], + ['fill("b", undefined, 3)', b => b.fill("b", undefined, 3)], + + // fill(string, encoding) has no end slot, so anything there is discarded. + ['fill("b", "utf8", 3)', b => b.fill("b", "utf8", 3)], + + // Shapes that already agreed, pinned so a regression in either direction shows up. + ["fill(0)", b => b.fill(0)], + ["fill(0, 1)", b => b.fill(0, 1)], + ["fill(0, 1, 3)", b => b.fill(0, 1, 3)], + ["fill(0, 1.5, 3)", b => b.fill(0, 1.5, 3)], + ["fill(0, -1, 3)", b => b.fill(0, -1, 3)], + ["fill(0, 1, 99)", b => b.fill(0, 1, 99)], + ["fill(0, NaN, 3)", b => b.fill(0, NaN, 3)], + ['fill(0, 1, 3, "bogus")', b => b.fill(0, 1, 3, "bogus")], + ['fill("b")', b => b.fill("b")], + ['fill("b", "utf8")', b => b.fill("b", "utf8")], + ['fill("b", 1)', b => b.fill("b", 1)], + ['fill("b", 1, 3)', b => b.fill("b", 1, 3)], + ['fill("b", 1, "utf8")', b => b.fill("b", 1, "utf8")], + ['fill("b", 1, 3, "utf8")', b => b.fill("b", 1, 3, "utf8")], + ['fill("b", "1", 3)', b => b.fill("b", "1", 3)], + ['fill("b", 1, "3")', b => b.fill("b", 1, "3")], + ['fill("b", "bogus")', b => b.fill("b", "bogus")], + ['fill("b", -1)', b => b.fill("b", -1)], + ['fill("abc", "hex")', b => b.fill("abc", "hex")], +]; + +const lines = []; +for (const [name, run] of cases) { + const buf = Buffer.alloc(5, 0xaa); + let result; + try { + run(buf); + result = "ok " + buf.toString("hex"); + } catch (err) { + // Only the class and code are printed: Node rewords messages far more + // often than it changes codes, and the exact messages are pinned + // separately in buffer.test.js. + result = "throws " + err.name + " " + err.code; + } + lines.push(name.padEnd(26) + " " + result); +} +process.stdout.write(lines.join("\n") + "\n"); diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 0648d8d02b5c..01924a839c8f 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -1,7 +1,8 @@ import { Buffer, SlowBuffer, isAscii, isUtf8, kMaxLength } from "buffer"; import { afterEach, beforeEach, describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, gc, isASAN, isDebug, withoutAggressiveGC } from "harness"; +import { bunEnv, bunExe, gc, isASAN, isDebug, nodeExe, withoutAggressiveGC } from "harness"; import { createHash } from "node:crypto"; +import { join } from "node:path"; import vm from "node:vm"; const BufferModule = await import("buffer"); @@ -3845,6 +3846,23 @@ describe("Buffer.fill offset/end argument handling", () => { it("never treats the 4-argument encoding as meaningful for a non-string value", () => { expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, 1, 3, "bogus"))).toEqual([0xaa, 0, 0, 0xaa, 0xaa]); }); + + // Differential test: the fixture enumerates every fill() argument shape and + // prints the resulting bytes or the thrown error class + code. Running it + // under Node.js and under Bun must produce byte-identical output. + it.skipIf(!nodeExe())("every fill() argument shape produces the same output in Node.js and Bun", async () => { + const fixture = join(import.meta.dir, "buffer-fill-args-fixture.js"); + async function run(exe) { + await using proc = Bun.spawn({ cmd: [exe, fixture], env: bunEnv, stdout: "pipe" }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + return { stdout, exitCode }; + } + const [bunRun, nodeRun] = await Promise.all([run(bunExe()), run(nodeExe())]); + expect(nodeRun.stdout).toContain("fill("); + expect(bunRun.stdout).toBe(nodeRun.stdout); + expect(nodeRun.exitCode).toBe(0); + expect(bunRun.exitCode).toBe(0); + }); }); describe("*Write methods with NaN/invalid offset and length", () => { From 43d4dfe22d69db75b39fa7a4a2d58e0f136d2c27 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:36:07 +0000 Subject: [PATCH 3/8] buffer: match Node for zero-argument and five-plus-argument fill() Two more argument-count shapes in fill()'s intake diverge from Node: Buffer.alloc(3, 0xaa).fill() // bun no-op ; node zero-fills Buffer.alloc(5, 0xaa).fill(0, 1, 3, "utf8", "x") // bun fills [0, 5); node fills [1, 3) The zero-argument early return skipped Node's path, where the undefined value reaches the numeric branch and coerces to 0. The argument switch had no case for five or more arguments, so every slot degraded to undefined. Use the bounds-checked callFrame->argument(n) for all four slots instead: it returns undefined for an absent argument and ignores trailing ones, matching Node's plain-JS signature. --- src/jsc/bindings/JSBuffer.cpp | 31 +++++++----------------- test/js/node/buffer-fill-args-fixture.js | 18 +++++++++++++- test/js/node/buffer.test.js | 24 ++++++++++++++++++ 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index c426c3e078cb..cc2e91c4692f 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -1313,11 +1313,9 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec auto& vm = JSC::getVM(lexicalGlobalObject); auto scope = DECLARE_THROW_SCOPE(vm); - if (callFrame->argumentCount() < 1) { - return JSValue::encode(castedThis); - } - - auto value = callFrame->uncheckedArgument(0); + // No early return for 0 arguments: Node's fill() forwards an undefined + // value into the numeric path, which zero-fills the whole buffer. + auto value = callFrame->argument(0); // Capture byteLength up front for two orthogonal purposes: // 1. The upper-bound argument to validateInteger(end) so `end > // buf.length` throws ERR_OUT_OF_RANGE with Node's wording and @@ -1335,23 +1333,12 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec size_t offset = 0; size_t end = limit; WebCore::BufferEncodingType encoding = WebCore::BufferEncodingType::utf8; - JSValue encodingValue = jsUndefined(); - JSValue offsetValue = jsUndefined(); - JSValue endValue = jsUndefined(); - - switch (callFrame->argumentCount()) { - case 4: - encodingValue = callFrame->uncheckedArgument(3); - [[fallthrough]]; - case 3: - endValue = callFrame->uncheckedArgument(2); - [[fallthrough]]; - case 2: - offsetValue = callFrame->uncheckedArgument(1); - [[fallthrough]]; - default: - break; - } + // argument() (not uncheckedArgument) so trailing positional arguments past + // the fourth are ignored like Node's plain-JS fill(value, offset, end, + // encoding) signature, instead of degrading every slot to undefined. + JSValue offsetValue = callFrame->argument(1); + JSValue endValue = callFrame->argument(2); + JSValue encodingValue = callFrame->argument(3); // Node's _fill only reinterprets a string offset/end as the encoding when the // fill value is itself a string; otherwise validateInteger below rejects them. diff --git a/test/js/node/buffer-fill-args-fixture.js b/test/js/node/buffer-fill-args-fixture.js index f98597791371..5705c7d8d6c8 100644 --- a/test/js/node/buffer-fill-args-fixture.js +++ b/test/js/node/buffer-fill-args-fixture.js @@ -26,6 +26,22 @@ const cases = [ // fill(string, encoding) has no end slot, so anything there is discarded. ['fill("b", "utf8", 3)', b => b.fill("b", "utf8", 3)], + // An undefined offset also takes the encoding slot in Node, shadowing an + // explicit 4th-argument encoding. A numeric offset keeps it. + ['fill("ab", undefined, undefined, "utf16le")', b => b.fill("ab", undefined, undefined, "utf16le")], + ['fill("a", undefined, undefined, "bogus")', b => b.fill("a", undefined, undefined, "bogus")], + ['fill("ab", 0, undefined, "utf16le")', b => b.fill("ab", 0, undefined, "utf16le")], + ['fill("a", 1, undefined, "bogus")', b => b.fill("a", 1, undefined, "bogus")], + + // No arguments: Node routes an undefined value into the numeric path (0). + ["fill()", b => b.fill()], + ["fill(undefined)", b => b.fill(undefined)], + + // Positional arguments past the fourth are ignored. + ['fill(0, 1, 3, "utf8", "x")', b => b.fill(0, 1, 3, "utf8", "x")], + ['fill("b", 1, 3, "utf8", "x")', b => b.fill("b", 1, 3, "utf8", "x")], + ['fill("ab", 0, 4, "utf16le", "x")', b => b.fill("ab", 0, 4, "utf16le", "x")], + // Shapes that already agreed, pinned so a regression in either direction shows up. ["fill(0)", b => b.fill(0)], ["fill(0, 1)", b => b.fill(0, 1)], @@ -61,6 +77,6 @@ for (const [name, run] of cases) { // separately in buffer.test.js. result = "throws " + err.name + " " + err.code; } - lines.push(name.padEnd(26) + " " + result); + lines.push(name.padEnd(44) + " " + result); } process.stdout.write(lines.join("\n") + "\n"); diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 01924a839c8f..7b810e8cffd5 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -3847,6 +3847,30 @@ describe("Buffer.fill offset/end argument handling", () => { expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, 1, 3, "bogus"))).toEqual([0xaa, 0, 0, 0xaa, 0xaa]); }); + it("zero-fills the whole buffer when called with no arguments", () => { + // Node forwards the undefined value into the numeric path, which coerces to 0. + expect(Array.from(Buffer.alloc(3, 0xaa).fill())).toEqual([0, 0, 0]); + expect(Array.from(Buffer.alloc(3, 0xaa).fill(undefined))).toEqual([0, 0, 0]); + }); + + it("ignores positional arguments past the fourth", () => { + expect(Array.from(Buffer.alloc(5, 0xaa).fill(0, 1, 3, "utf8", "x"))).toEqual([0xaa, 0, 0, 0xaa, 0xaa]); + expect(Buffer.alloc(5, 0xaa).fill("ab", 0, 4, "utf16le", "x").toString("hex")).toBe("61006200aa"); + }); + + it("lets an undefined offset shadow an explicit 4th-argument encoding, like Node", () => { + // Node's _fill assigns `encoding = offset` whenever offset is undefined or + // a string, so fill(str, undefined, ..., encoding) falls back to utf8 and a + // bogus 4th-argument encoding is never even validated. + expect(Buffer.alloc(4, 0xaa).fill("ab", undefined, undefined, "utf16le").toString("hex")).toBe("61626162"); + expect(Buffer.alloc(4, 0xaa).fill("a", undefined, undefined, "bogus").toString("hex")).toBe("61616161"); + // With a numeric offset the explicit encoding is honored. + expect(Buffer.alloc(4, 0xaa).fill("ab", 0, undefined, "utf16le").toString("hex")).toBe("61006200"); + expect(() => Buffer.alloc(4).fill("a", 1, undefined, "bogus")).toThrow( + expect.objectContaining({ code: "ERR_UNKNOWN_ENCODING" }), + ); + }); + // Differential test: the fixture enumerates every fill() argument shape and // prints the resulting bytes or the thrown error class + code. Running it // under Node.js and under Bun must produce byte-identical output. From 83bc178e9d4c7c9e62fdbf2e3d0b1954d3e2279e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 02:11:49 +0000 Subject: [PATCH 4/8] buffer: treat a null encoding as absent in fill() and alloc() Node's normalizeEncoding tests `enc == null`, so an explicit null encoding is the same as an absent one and defaults to utf8. Bun's fill() and Buffer.alloc() gated on isUndefined(), so null reached parseEncoding and threw: Buffer.alloc(5, 0xaa).fill("a", 1, 3, null) // bun ERR_INVALID_ARG_TYPE // node Buffer.alloc(3, "a", null) // bun ERR_INVALID_ARG_TYPE // node <61 61 61> buf.toString(null) is unchanged: Node routes it through getEncodingOps, which does throw ERR_UNKNOWN_ENCODING for a null encoding, so the null handling belongs at these two call sites and not inside parseEncoding. --- src/jsc/bindings/JSBuffer.cpp | 9 ++++++--- test/js/node/buffer-fill-args-fixture.js | 4 ++++ test/js/node/buffer.test.js | 9 +++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index cc2e91c4692f..6634d94cdc1d 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -662,7 +662,9 @@ static JSC::EncodedJSValue jsBufferConstructorFunction_allocBody(JSC::JSGlobalOb WebCore::BufferEncodingType encoding = WebCore::BufferEncodingType::utf8; if (callFrame->argumentCount() > 2) { EnsureStillAliveScope arg2 = callFrame->uncheckedArgument(2); - if (!arg2.value().isUndefined()) { + // A null encoding is the same as an absent one (Node's + // normalizeEncoding tests `enc == null`), not an error. + if (!arg2.value().isUndefinedOrNull()) { encoding = parseEncoding(scope, lexicalGlobalObject, arg2.value(), true); RETURN_IF_EXCEPTION(scope, {}); } @@ -1361,8 +1363,9 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec // encoding error wins. parseEncoding is also the first // user-JS-visible call: `toString` on an object encoding can detach // or resize castedThis; the post-coercion clamp further down reads - // byteLength() once more to catch any such effect. - if (!encodingValue.isUndefined() && value.isString()) { + // byteLength() once more to catch any such effect. A null encoding is the + // same as an absent one (Node's normalizeEncoding tests `enc == null`). + if (!encodingValue.isUndefinedOrNull() && value.isString()) { encoding = parseEncoding(scope, lexicalGlobalObject, encodingValue, true); RETURN_IF_EXCEPTION(scope, {}); } diff --git a/test/js/node/buffer-fill-args-fixture.js b/test/js/node/buffer-fill-args-fixture.js index 5705c7d8d6c8..a8f4307a6035 100644 --- a/test/js/node/buffer-fill-args-fixture.js +++ b/test/js/node/buffer-fill-args-fixture.js @@ -33,6 +33,10 @@ const cases = [ ['fill("ab", 0, undefined, "utf16le")', b => b.fill("ab", 0, undefined, "utf16le")], ['fill("a", 1, undefined, "bogus")', b => b.fill("a", 1, undefined, "bogus")], + // A null encoding is the same as an absent one (Node's normalizeEncoding). + ['fill("a", 1, 3, null)', b => b.fill("a", 1, 3, null)], + ['fill("a", undefined, undefined, null)', b => b.fill("a", undefined, undefined, null)], + // No arguments: Node routes an undefined value into the numeric path (0). ["fill()", b => b.fill()], ["fill(undefined)", b => b.fill(undefined)], diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 7b810e8cffd5..f9ba8a818742 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -3871,6 +3871,15 @@ describe("Buffer.fill offset/end argument handling", () => { ); }); + it("treats a null encoding like an absent one, as Node's normalizeEncoding does", () => { + expect(Buffer.alloc(5, 0xaa).fill("a", 1, 3, null).toString("hex")).toBe("aa6161aaaa"); + expect(Buffer.alloc(3, "a", null).toString("hex")).toBe("616161"); + // toString goes through Node's getEncodingOps, not normalizeEncoding, so a + // null encoding there is still ERR_UNKNOWN_ENCODING. Pins that the null + // handling lives at the fill/alloc gates, not inside parseEncoding. + expect(() => Buffer.from("ab").toString(null)).toThrow(expect.objectContaining({ code: "ERR_UNKNOWN_ENCODING" })); + }); + // Differential test: the fixture enumerates every fill() argument shape and // prints the resulting bytes or the thrown error class + code. Running it // under Node.js and under Bun must produce byte-identical output. From 9c96ad014fc82f5c1ec86c101e605d90dc51f0b8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 02:47:41 +0000 Subject: [PATCH 5/8] buffer: treat an empty-string encoding as absent in fill() and alloc() Node's normalizeEncoding also returns utf8 for an empty string (the slowCases default branch), so the treat-as-absent set is exactly undefined, null, and "". The fill() and Buffer.alloc() encoding gates only skipped null and undefined, so "" reached parseEncoding and threw: Buffer.alloc(5, 0xaa).fill("a", 1, 3, "") // bun ERR_UNKNOWN_ENCODING // node Buffer.alloc(5, 0xaa).fill("a", "") // bun ERR_UNKNOWN_ENCODING // node <61 61 61 61 61> Buffer.alloc(3, "a", "") // bun ERR_UNKNOWN_ENCODING // node <61 61 61> Extract the two gates' shared predicate into isAbsentEncoding, which now covers all three values. parseEncoding is unchanged: buf.toString("") and buf.toString(null) must keep throwing ERR_UNKNOWN_ENCODING, because Node routes toString through getEncodingOps, which rejects both. Buffer.from("ab", "") has the same symptom but through a different Node helper (fromString treats any non-string or empty-string encoding as utf8, and Bun already matches it for every non-string value); its gate is outside this file's fill/alloc path and is left for its own change. --- src/jsc/bindings/JSBuffer.cpp | 17 +++++++++++------ test/js/node/buffer-fill-args-fixture.js | 6 +++++- test/js/node/buffer.test.js | 18 +++++++++++++++--- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 6634d94cdc1d..3a0479a70eb6 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -293,6 +293,14 @@ static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JS return encoded.value(); } +// Node's normalizeEncoding (lib/internal/util.js) treats undefined, null, and +// the empty string as an absent encoding (utf8). buf.toString rejects all three +// (getEncodingOps), so this lives at the fill/alloc gates, not in parseEncoding. +static bool isAbsentEncoding(JSC::JSValue value) +{ + return value.isUndefinedOrNull() || (value.isString() && asString(value)->length() == 0); +} + // Matches Node's validateOffset (lib/buffer.js), which is validateInteger and // therefore renders its range as ">= min && <= max", unlike boundsError's // ">= min and <= max". @@ -662,9 +670,7 @@ static JSC::EncodedJSValue jsBufferConstructorFunction_allocBody(JSC::JSGlobalOb WebCore::BufferEncodingType encoding = WebCore::BufferEncodingType::utf8; if (callFrame->argumentCount() > 2) { EnsureStillAliveScope arg2 = callFrame->uncheckedArgument(2); - // A null encoding is the same as an absent one (Node's - // normalizeEncoding tests `enc == null`), not an error. - if (!arg2.value().isUndefinedOrNull()) { + if (!isAbsentEncoding(arg2.value())) { encoding = parseEncoding(scope, lexicalGlobalObject, arg2.value(), true); RETURN_IF_EXCEPTION(scope, {}); } @@ -1363,9 +1369,8 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec // encoding error wins. parseEncoding is also the first // user-JS-visible call: `toString` on an object encoding can detach // or resize castedThis; the post-coercion clamp further down reads - // byteLength() once more to catch any such effect. A null encoding is the - // same as an absent one (Node's normalizeEncoding tests `enc == null`). - if (!encodingValue.isUndefinedOrNull() && value.isString()) { + // byteLength() once more to catch any such effect. + if (!isAbsentEncoding(encodingValue) && value.isString()) { encoding = parseEncoding(scope, lexicalGlobalObject, encodingValue, true); RETURN_IF_EXCEPTION(scope, {}); } diff --git a/test/js/node/buffer-fill-args-fixture.js b/test/js/node/buffer-fill-args-fixture.js index a8f4307a6035..940de599be69 100644 --- a/test/js/node/buffer-fill-args-fixture.js +++ b/test/js/node/buffer-fill-args-fixture.js @@ -33,9 +33,13 @@ const cases = [ ['fill("ab", 0, undefined, "utf16le")', b => b.fill("ab", 0, undefined, "utf16le")], ['fill("a", 1, undefined, "bogus")', b => b.fill("a", 1, undefined, "bogus")], - // A null encoding is the same as an absent one (Node's normalizeEncoding). + // A null or empty-string encoding is the same as an absent one: Node's + // normalizeEncoding returns utf8 for undefined, null, and "". ['fill("a", 1, 3, null)', b => b.fill("a", 1, 3, null)], ['fill("a", undefined, undefined, null)', b => b.fill("a", undefined, undefined, null)], + ['fill("a", 1, 3, "")', b => b.fill("a", 1, 3, "")], + ['fill("a", "")', b => b.fill("a", "")], + ['fill("a", 1, "")', b => b.fill("a", 1, "")], // No arguments: Node routes an undefined value into the numeric path (0). ["fill()", b => b.fill()], diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index f9ba8a818742..a5081bb5b2a7 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -3871,13 +3871,25 @@ describe("Buffer.fill offset/end argument handling", () => { ); }); - it("treats a null encoding like an absent one, as Node's normalizeEncoding does", () => { + it("treats a null or empty-string encoding like an absent one, as Node's normalizeEncoding does", () => { + // normalizeEncoding returns utf8 for exactly undefined, null, and "". expect(Buffer.alloc(5, 0xaa).fill("a", 1, 3, null).toString("hex")).toBe("aa6161aaaa"); + expect(Buffer.alloc(5, 0xaa).fill("a", 1, 3, "").toString("hex")).toBe("aa6161aaaa"); + // A string "" in the offset or end slot becomes the encoding first, then + // that encoding is treated as absent. + expect(Buffer.alloc(5, 0xaa).fill("a", "").toString("hex")).toBe("6161616161"); + expect(Buffer.alloc(5, 0xaa).fill("a", 1, "").toString("hex")).toBe("aa61616161"); expect(Buffer.alloc(3, "a", null).toString("hex")).toBe("616161"); + expect(Buffer.alloc(3, "a", "").toString("hex")).toBe("616161"); // toString goes through Node's getEncodingOps, not normalizeEncoding, so a - // null encoding there is still ERR_UNKNOWN_ENCODING. Pins that the null - // handling lives at the fill/alloc gates, not inside parseEncoding. + // null or empty-string encoding there is still ERR_UNKNOWN_ENCODING. Pins + // that the handling lives at the fill/alloc gates, not inside parseEncoding. expect(() => Buffer.from("ab").toString(null)).toThrow(expect.objectContaining({ code: "ERR_UNKNOWN_ENCODING" })); + expect(() => Buffer.from("ab").toString("")).toThrow(expect.objectContaining({ code: "ERR_UNKNOWN_ENCODING" })); + // A String object is not a string primitive, so it is not "absent". + expect(() => Buffer.alloc(3, "a", new String(""))).toThrow( + expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE" }), + ); }); // Differential test: the fixture enumerates every fill() argument shape and From 97c4054eb3620a466c5ebffac3a400b5508efbae Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 05:09:08 +0000 Subject: [PATCH 6/8] ci: retrigger From b721d06cebe20e4e559cfa551a0a32850218a16a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 22:14:15 +0000 Subject: [PATCH 7/8] buffer: resolve the encoding argument to a JSString once isAbsentEncoding checked the value with asString() and then parseEncoding coerced the same value again with toStringOrNull(). Replace it with resolveEncodingString, which returns std::optional: nullopt for Node's absent set (undefined, null, the primitive empty string) and otherwise the encoding resolved to a JSString exactly once. parseEncoding gains an overload taking that JSString directly, and the existing JSValue overload (used by toString and write) delegates to it unchanged. No behavior change: the 48-shape Node/Bun differential, the full buffer.test.js suite, and the detaching-encoding-toString TOCTOU suite all pass unchanged. --- src/jsc/bindings/JSBuffer.cpp | 51 +++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index 3a0479a70eb6..ce71ff0318d1 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -274,11 +274,11 @@ static int normalizeCompareVal(int val, size_t a_length, size_t b_length) return val; } -static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JSGlobalObject* lexicalGlobalObject, JSValue arg, bool validateUnknown) +// `arg` is the original, uncoerced encoding argument; it only shapes the +// ERR_INVALID_ARG_TYPE message. `argString` is its resolved JSString. +static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JSGlobalObject* lexicalGlobalObject, JSString* argString, JSValue arg, bool validateUnknown) { - auto arg_ = arg.toStringOrNull(lexicalGlobalObject); - RETURN_IF_EXCEPTION(scope, {}); - const auto& view = arg_->view(lexicalGlobalObject); + const auto& view = argString->view(lexicalGlobalObject); std::optional encoded = parseEnumerationFromView(view); if (!encoded) [[unlikely]] { @@ -293,12 +293,29 @@ static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JS return encoded.value(); } -// Node's normalizeEncoding (lib/internal/util.js) treats undefined, null, and -// the empty string as an absent encoding (utf8). buf.toString rejects all three -// (getEncodingOps), so this lives at the fill/alloc gates, not in parseEncoding. -static bool isAbsentEncoding(JSC::JSValue value) +static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JSGlobalObject* lexicalGlobalObject, JSValue arg, bool validateUnknown) { - return value.isUndefinedOrNull() || (value.isString() && asString(value)->length() == 0); + auto arg_ = arg.toStringOrNull(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); + return parseEncoding(scope, lexicalGlobalObject, arg_, arg, validateUnknown); +} + +// Node's normalizeEncoding treats undefined, null, and the primitive empty string +// as an absent encoding (utf8); buf.toString (Node's getEncodingOps) rejects all +// three, so this lives at the fill/alloc gates and hands back the resolved JSString. +static std::optional resolveEncodingString(JSC::ThrowScope& scope, JSC::JSGlobalObject* lexicalGlobalObject, JSValue value) +{ + if (value.isUndefinedOrNull()) + return std::nullopt; + if (value.isString()) { + auto* str = asString(value); + if (!str->length()) + return std::nullopt; + return str; + } + auto* str = value.toStringOrNull(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); + return str; } // Matches Node's validateOffset (lib/buffer.js), which is validateInteger and @@ -670,8 +687,10 @@ static JSC::EncodedJSValue jsBufferConstructorFunction_allocBody(JSC::JSGlobalOb WebCore::BufferEncodingType encoding = WebCore::BufferEncodingType::utf8; if (callFrame->argumentCount() > 2) { EnsureStillAliveScope arg2 = callFrame->uncheckedArgument(2); - if (!isAbsentEncoding(arg2.value())) { - encoding = parseEncoding(scope, lexicalGlobalObject, arg2.value(), true); + std::optional encodingString = resolveEncodingString(scope, lexicalGlobalObject, arg2.value()); + RETURN_IF_EXCEPTION(scope, {}); + if (encodingString) { + encoding = parseEncoding(scope, lexicalGlobalObject, *encodingString, arg2.value(), true); RETURN_IF_EXCEPTION(scope, {}); } } @@ -1366,13 +1385,17 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec // Node validates encoding before either `validateInteger` call, so // `fill("a", 0, buf.length + 1, "bogus")` and `fill("a", -1, 0, // "bogus")` throw ERR_UNKNOWN_ENCODING (not ERR_OUT_OF_RANGE) — the - // encoding error wins. parseEncoding is also the first + // encoding error wins. Resolving the encoding is also the first // user-JS-visible call: `toString` on an object encoding can detach // or resize castedThis; the post-coercion clamp further down reads // byteLength() once more to catch any such effect. - if (!isAbsentEncoding(encodingValue) && value.isString()) { - encoding = parseEncoding(scope, lexicalGlobalObject, encodingValue, true); + if (value.isString()) { + std::optional encodingString = resolveEncodingString(scope, lexicalGlobalObject, encodingValue); RETURN_IF_EXCEPTION(scope, {}); + if (encodingString) { + encoding = parseEncoding(scope, lexicalGlobalObject, *encodingString, encodingValue, true); + RETURN_IF_EXCEPTION(scope, {}); + } } // ── 2. Pure offset / end coercion (no user JS) ────────────────────── From ba95354e5b01802fcb0efdc1792e1cd2750cc7db Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 30 Jun 2026 00:25:44 +0000 Subject: [PATCH 8/8] buffer: use toString instead of toStringOrNull when resolving the encoding Both JSC variants throw the pending exception; toStringOrNull only returns null instead of the empty string on the throwing path (see the comment on JSValue::toStringOrNull in JSCJSValue.h). With toString, an engaged std::optional can never hold a null pointer. Add a test pinning that an exception thrown from an object encoding's toString propagates out of fill() and Buffer.alloc(). --- src/jsc/bindings/JSBuffer.cpp | 6 ++++-- test/js/node/buffer.test.js | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index ce71ff0318d1..70255deff997 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -295,7 +295,7 @@ static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JS static WebCore::BufferEncodingType parseEncoding(JSC::ThrowScope& scope, JSC::JSGlobalObject* lexicalGlobalObject, JSValue arg, bool validateUnknown) { - auto arg_ = arg.toStringOrNull(lexicalGlobalObject); + auto arg_ = arg.toString(lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, {}); return parseEncoding(scope, lexicalGlobalObject, arg_, arg, validateUnknown); } @@ -313,7 +313,9 @@ static std::optional resolveEncodingString(JSC::ThrowScope& scope, JS return std::nullopt; return str; } - auto* str = value.toStringOrNull(lexicalGlobalObject); + // toString() can run user JS and throw; unlike toStringOrNull it returns the + // empty string on the exception path, so an engaged optional never holds null. + auto* str = value.toString(lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, {}); return str; } diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index a5081bb5b2a7..130d3361687b 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -3892,6 +3892,19 @@ describe("Buffer.fill offset/end argument handling", () => { ); }); + // Bun coerces an object encoding via toString (Node rejects a non-string + // encoding up front with ERR_INVALID_ARG_TYPE instead). An exception thrown + // from that toString must surface, not degrade to an empty or null string. + it("propagates an exception thrown from an object encoding's toString", () => { + const boom = { + toString() { + throw new Error("boom"); + }, + }; + expect(() => Buffer.alloc(4, 0xaa).fill("a", 0, 4, boom)).toThrow("boom"); + expect(() => Buffer.alloc(4, "a", boom)).toThrow("boom"); + }); + // Differential test: the fixture enumerates every fill() argument shape and // prints the resulting bytes or the thrown error class + code. Running it // under Node.js and under Bun must produce byte-identical output.