From 63df7755a82d9b38f1ef91f448ca26861ec42415 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:38:19 +0000 Subject: [PATCH] node:fs: write UTF-8 when fs.write gets an unknown encoding name fs.writeSync(fd, "XY", null, "bogus") threw ERR_INVALID_ARG_VALUE and wrote nothing. fs.write and filehandle.write did the same. Node.js through v26 writes the string as UTF-8. For a null or undefined position, Bun 1.3.14 wrote UTF-8 too, and 1.4.0 started to throw (#32813). args::Write::from_js parsed the encoding with Encoding::assert, the port of Node's assertEncoding. Node's WriteString reads that argument with ParseEncoding(args[3], UTF8), which never throws. The parser now does the same: a primitive string that names a known encoding selects it, and every other value means UTF-8. Node.js v27 rejects an unknown name in its JS validateEncoding (nodejs/node#66247). That check is not part of this change. --- src/runtime/node/node_fs.rs | 6 +- .../node/fs/fs-write-unknown-encoding.test.ts | 191 ++++++++++++++++++ 2 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 test/js/node/fs/fs-write-unknown-encoding.test.ts diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index a70451774870..0e1ef57f55b9 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4223,8 +4223,10 @@ pub(crate) mod args { break 'parse; }; current = next; - if current.is_string() { - args.encoding = Encoding::assert(current, ctx, args.encoding)?; + // Node's ParseEncoding(args[3], UTF8) never throws: https://github.com/nodejs/node/blob/v26.12.1/src/node_file.cc#L3204 + if current.is_string_literal() { + args.encoding = + Encoding::from_js(current, ctx)?.unwrap_or(args.encoding); arguments.eat(); // `bv` was converted to UTF-8 before the encoding // argument was parsed; re-encode it now. Node diff --git a/test/js/node/fs/fs-write-unknown-encoding.test.ts b/test/js/node/fs/fs-write-unknown-encoding.test.ts new file mode 100644 index 000000000000..59919dc66a45 --- /dev/null +++ b/test/js/node/fs/fs-write-unknown-encoding.test.ts @@ -0,0 +1,191 @@ +/** + * A string write with an encoding argument that does not name an encoding + * writes UTF-8. Node's binding reads the argument with + * ParseEncoding(args[3], UTF8), which returns UTF8 for an unknown name and for + * a value that is not a primitive string: + * https://github.com/nodejs/node/blob/v26.12.1/src/node_file.cc#L3204 + * https://github.com/nodejs/node/blob/v26.12.1/src/api/encoding.cc#L125-L136 + * + * This file uses node:test/node:assert so the identical file also runs under + * `node`, which is where the expected values come from: Node.js v26.12.1 + * passes every test. It cannot import from "harness" for the same reason, so + * it manages its own temporary directory. + * + * Node.js v27 rejects an unknown encoding with ERR_UNKNOWN_ENCODING in its JS + * validateEncoding (https://github.com/nodejs/node/pull/66247), so this file + * fails under that Node.js. When Bun reports v27, port that check and change + * the expected values here. + */ +import assert from "node:assert"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { after, describe, test } from "node:test"; +import { promisify } from "node:util"; + +const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fs-write-unknown-encoding-")); +const file = path.join(dir, "out.bin"); +after(() => fs.rmSync(dir, { recursive: true, force: true })); + +// "é" tells UTF-8 (c3 a9) from latin1 (e9), and it ends a hex decode after "ab". +const data = "ab\u00e9d"; +const utf8 = [0x61, 0x62, 0xc3, 0xa9, 0x64]; +const utf16le = [0x61, 0, 0x62, 0, 0xe9, 0, 0x64, 0]; + +const encodings: [label: string, encoding: unknown, bytes: number[]][] = [ + ["'bogus'", "bogus", utf8], + ["'BOGUS'", "BOGUS", utf8], + ["'he'", "he", utf8], + ["'hexx'", "hexx", utf8], + ["' hex'", " hex", utf8], + ["''", "", utf8], + ["'\\u00fctf8'", "\u00fctf8", utf8], + ["'\\u{1F600}'", "\u{1F600}", utf8], + ["a name of 300 characters", Buffer.alloc(300, "x").toString(), utf8], + // Only a primitive string names an encoding. + ["new String('hex')", new String("hex"), utf8], + ["new String('bogus')", new String("bogus"), utf8], + ["{ toString: () => 'hex' }", { toString: () => "hex" }, utf8], + ["['hex']", ["hex"], utf8], + // A known name is still matched, in any case and from a 16-bit string. + ["'HeX'", "HeX", [0xab]], + ["'hex' decoded from UTF-16", Buffer.from("hex", "utf16le").toString("utf16le"), [0xab]], + ["'utf-16le'", "utf-16le", utf16le], + ["'UTF-16LE'", "UTF-16LE", utf16le], +]; +// The async forms share the argument parser with writeSync, so they get fewer rows. +const unknown: [label: string, encoding: unknown][] = [ + ["'bogus'", "bogus"], + ["'hexx'", "hexx"], + ["new String('hex')", new String("hex")], +]; + +// Calls `call` with a fresh fd for `file`. +async function onFd(flags: string, call: (fd: number) => T | Promise) { + const fd = fs.openSync(file, flags); + try { + return await call(fd); + } finally { + fs.closeSync(fd); + } +} + +describe("fs.writeSync(fd, string, position, encoding)", () => { + for (const [label, encoding, bytes] of encodings) { + test(`encoding ${label}`, async () => { + // Bun 1.3.14 wrote UTF-8 for null and undefined. Bun 1.4.0 threw for all three. + for (const position of [0, null, undefined]) { + const written = await onFd("w", fd => (fs.writeSync as Function)(fd, data, position, encoding)); + assert.deepStrictEqual( + { position, written, bytes: [...fs.readFileSync(file)] }, + { position, written: bytes.length, bytes }, + ); + } + }); + } + + test("an empty string writes nothing", async () => { + assert.strictEqual(await onFd("w", fd => (fs.writeSync as Function)(fd, "", null, "bogus")), 0); + assert.deepStrictEqual([...fs.readFileSync(file)], []); + }); + + test("a write that fails throws the error of the write", async () => { + fs.writeFileSync(file, "...."); + await onFd("r", fd => { + assert.throws(() => (fs.writeSync as Function)(fd, "ab", null, "bogus"), { code: "EBADF", syscall: "write" }); + }); + assert.strictEqual(fs.readFileSync(file, "latin1"), "...."); + }); +}); + +describe("fs.write(fd, string, position, encoding, callback)", () => { + // Resolves with the arguments of the callback, and with whether fs.write() had returned by then. + function write(fd: number, ...args: unknown[]) { + const { promise, resolve } = Promise.withResolvers(); + let returned = false; + (fs.write as Function)(fd, ...args, (err: any, ...rest: unknown[]) => + resolve([err?.code ?? err, ...rest, { afterReturn: returned }]), + ); + returned = true; + return promise; + } + + for (const [label, encoding] of unknown) { + for (const position of [null, 0]) { + test(`encoding ${label}, position ${position}`, async () => { + assert.deepStrictEqual(await onFd("w", fd => write(fd, data, position, encoding)), [ + null, + utf8.length, + data, + { afterReturn: true }, + ]); + assert.deepStrictEqual([...fs.readFileSync(file)], utf8); + }); + } + } + + test("an empty string writes nothing", async () => { + assert.deepStrictEqual(await onFd("w", fd => write(fd, "", null, "bogus")), [null, 0, "", { afterReturn: true }]); + assert.deepStrictEqual([...fs.readFileSync(file)], []); + }); + + test("a write that fails gives the error of the write to the callback", async () => { + fs.writeFileSync(file, "...."); + assert.deepStrictEqual(await onFd("r", fd => write(fd, "ab", null, "bogus")), [ + "EBADF", + 0, + "ab", + { afterReturn: true }, + ]); + assert.strictEqual(fs.readFileSync(file, "latin1"), "...."); + }); +}); + +// `write` makes one string write on a fresh fd for `file` and resolves with `{ bytesWritten, buffer }`. +function promiseForm(name: string, write: (flags: string, ...args: unknown[]) => Promise) { + describe(name, () => { + for (const [label, encoding] of unknown) { + for (const position of [null, 0]) { + test(`encoding ${label}, position ${position}`, async () => { + // Node's FileHandle result has a null prototype. The copy compares the properties only. + assert.deepStrictEqual( + { ...(await write("w", data, position, encoding)) }, + { bytesWritten: utf8.length, buffer: data }, + ); + assert.deepStrictEqual([...fs.readFileSync(file)], utf8); + }); + } + } + + test("an empty string writes nothing", async () => { + assert.deepStrictEqual({ ...(await write("w", "", null, "bogus")) }, { bytesWritten: 0, buffer: "" }); + assert.deepStrictEqual([...fs.readFileSync(file)], []); + }); + + test("a write that fails rejects with the error of the write", async () => { + fs.writeFileSync(file, "...."); + await assert.rejects(write("r", "ab", null, "bogus"), { code: "EBADF", syscall: "write" }); + assert.strictEqual(fs.readFileSync(file, "latin1"), "...."); + }); + }); +} + +promiseForm("util.promisify(fs.write)(fd, string, position, encoding)", (flags, ...args) => + onFd(flags, fd => (promisify(fs.write) as Function)(fd, ...args)), +); + +promiseForm("filehandle.write(string, position, encoding)", async (flags, ...args) => { + const handle = await fs.promises.open(file, flags); + try { + return await (handle.write as Function)(...args); + } finally { + await handle.close(); + } +}); + +// Node.js has no fs.promises.write. Bun's reaches the argument parser with no check in JS before it. +if (typeof (fs.promises as any).write === "function") { + promiseForm("fs.promises.write(fd, string, position, encoding)", (flags, ...args) => + onFd(flags, async fd => ({ bytesWritten: await (fs.promises as any).write(fd, ...args), buffer: args[0] })), + ); +}