From 50e2a6cf835643638e154e095949a6fecdaaf27b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 05:08:44 +0000 Subject: [PATCH 1/9] Stop reading user-mutable built-ins on the fs, child_process, dns, and CJS loader paths Internal modules read several built-ins live at call time. A user patch that changes results (a buggy polyfill, an instrumentation shim) then changes what the API does, silently. Node.js is immune because its internals use primordials captured at bootstrap. fs - cpSync walked each directory with for...of, so a patched Array.prototype[Symbol.iterator] dropped entries from the copy. Iterate by index. - fs.promises readFile/writeFile/appendFile forwarded their arguments with a spread, so the same patch dropped the encoding (readFile returned a Buffer for 'utf8') or the data. Take named parameters. - The callback fs API resolved every native promise with .then(), so a patched Promise.prototype.then saw (and could change) the file descriptor of fs.open() before createReadStream/createWriteStream used it. Use the private $then, and the same in internal/fs/streams for the FileSink promises. child_process - spawn/spawnSync built cmd with the live Array.prototype.slice plus a spread, so patched slice or iterator changed the child's argv. Copy spawnargs with the captured slice and overwrite index 0. fork's argv, the shell command string, and the env loop no longer spread or for...of either. dns / net - isIP() used RegExp.prototype.test, so a patched test() made dns.lookup() return the hostname as the address. Use the captured RegExp.prototype.exec like the other internal regex checks. CJS loader - require() picked the loader with the live String.prototype startsWith/endsWith/indexOf: a patched endsWith handed any file to dlopen, a patched startsWith broke require("node:*"). Use the engine's $stringSubstring and $stringIndexOfInternal link-time constants. --- src/js/builtins.d.ts | 4 + src/js/builtins/CommonJS.ts | 15 ++- src/js/internal/fs/cp-sync.ts | 5 +- src/js/internal/fs/streams.ts | 16 +-- src/js/internal/net/isIP.ts | 33 +++-- src/js/node/child_process.ts | 24 +++- src/js/node/fs.promises.ts | 23 ++-- src/js/node/fs.ts | 98 +++++++------- .../bun/resolve/require-primordials.test.ts | 41 ++++++ .../child_process-primordials.test.ts | 75 +++++++++++ test/js/node/fs/fs-primordials.test.ts | 126 ++++++++++++++++++ test/js/node/net/net-isip-primordials.test.ts | 44 ++++++ 12 files changed, 409 insertions(+), 95 deletions(-) create mode 100644 test/js/bun/resolve/require-primordials.test.ts create mode 100644 test/js/node/child_process/child_process-primordials.test.ts create mode 100644 test/js/node/fs/fs-primordials.test.ts create mode 100644 test/js/node/net/net-isip-primordials.test.ts diff --git a/src/js/builtins.d.ts b/src/js/builtins.d.ts index 6a79984312ae..cc99a1aaa504 100644 --- a/src/js/builtins.d.ts +++ b/src/js/builtins.d.ts @@ -540,6 +540,10 @@ declare function $ERR_HTTP2_PING_CANCEL(): Error; declare function $toClass(fn: Function, name: string, base?: Function | undefined | null); declare function $min(a: number, b: number): number; +/** The engine's own `String.prototype.substring`; user changes to the prototype do not reach it. */ +declare var $stringSubstring: typeof String.prototype.substring; +/** The engine's own `String.prototype.indexOf`; user changes to the prototype do not reach it. */ +declare var $stringIndexOfInternal: typeof String.prototype.indexOf; interface Map { $get: typeof Map.prototype.get; diff --git a/src/js/builtins/CommonJS.ts b/src/js/builtins/CommonJS.ts index 31ba228b0f5d..37576335dc85 100644 --- a/src/js/builtins/CommonJS.ts +++ b/src/js/builtins/CommonJS.ts @@ -17,7 +17,9 @@ $overriddenName = "require"; $visibility = "Private"; export function overridableRequire(this: JSCommonJSModule, originalId: string, options?: { paths?: string[] }) { const id = $resolveSync(originalId, this.filename, false, false, options ? options.paths : undefined, this, options); - if (id.startsWith("node:")) { + // $stringSubstring / $stringIndexOfInternal are the engine's own String + // methods; a user-patched String.prototype must not pick the loader. + if ($stringSubstring.$call(id, 0, 5) === "node:") { if (id !== originalId) { // A terrible special case where Node.js allows non-prefixed built-ins to // read the require cache. Though they never write to it, which is so silly. @@ -62,8 +64,9 @@ export function overridableRequire(this: JSCommonJSModule, originalId: string, o // A resolved id may carry a `?query` suffix (part of the module cache key); // match the native-addon extension against the path portion only. - const queryIndex = id.indexOf("?"); - if (queryIndex === -1 ? id.endsWith(".node") : id.endsWith(".node", queryIndex)) { + const queryIndex = $stringIndexOfInternal.$call(id, "?"); + const pathEnd = queryIndex === -1 ? id.length : queryIndex; + if (pathEnd >= 5 && $stringSubstring.$call(id, pathEnd - 5, pathEnd) === ".node") { return $internalRequire(id, this); } @@ -156,9 +159,9 @@ export function internalRequire(id: string, parent: JSCommonJSModule) { $assert($requireMap.$get(id) === undefined, "Module " + JSON.stringify(id) + " should not be in the map"); // `id` keys the module cache and may carry a `?query` suffix; // `process.dlopen` needs the on-disk path. - const queryIndex = id.indexOf("?"); - const filename = queryIndex === -1 ? id : id.substring(0, queryIndex); - $assert(filename.endsWith(".node")); + const queryIndex = $stringIndexOfInternal.$call(id, "?"); + const filename = queryIndex === -1 ? id : $stringSubstring.$call(id, 0, queryIndex); + $assert($stringSubstring.$call(filename, filename.length - 5) === ".node"); const module = $createCommonJSModule(id, {}, true, parent); process.dlopen(module, filename); diff --git a/src/js/internal/fs/cp-sync.ts b/src/js/internal/fs/cp-sync.ts index be4de692fcbb..03a8bb66c9b8 100644 --- a/src/js/internal/fs/cp-sync.ts +++ b/src/js/internal/fs/cp-sync.ts @@ -442,8 +442,9 @@ function mkDirAndCopy(srcMode, src, dest, opts) { } function copyDir(src, dest, opts) { - for (const dirent of readdirSync(src, { withFileTypes: true })) { - const { name } = dirent; + const dirents = readdirSync(src, { withFileTypes: true }); + for (let i = 0; i < dirents.length; i++) { + const { name } = dirents[i]; const srcItem = join(src, name); const destItem = join(dest, name); const { destStat, skipped } = checkPathsSync(srcItem, destItem, opts); diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 365c24987f53..89ea82ae9d31 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -562,11 +562,11 @@ function _write(data, encoding, cb) { const maybePromise = fileSink.write(data); if ($isPromise(maybePromise)) { maybePromise - .then(() => { + .$then(() => { this.emit("drain"); // Emit drain event cb(null); }) - .catch(cb); + .$then(undefined, cb); return false; // Indicate backpressure } else { cb(null); @@ -606,7 +606,7 @@ function underscoreWriteFast(this: FSStream, data: any, encoding: any, cb: any) const maybePromise = fileSink.write(data); if ($isPromise(maybePromise)) { - maybePromise.then( + maybePromise.$then( () => { if (cb) cb(null); this.emit("drain"); @@ -655,7 +655,7 @@ function writeFast(this: FSStream, data: any, encoding: any, cb: any) { if ($isPromise(maybePromise)) { // Two-arg then(): a throw from the fulfillment handler must not be // mistaken for a write failure. - maybePromise.then( + maybePromise.$then( () => { this.emit("drain"); // Emit drain event cb(null); @@ -699,11 +699,11 @@ writeStreamPrototype._writev = function (data, cb) { const maybePromise = fileSink.write(Buffer.concat(chunks)); if ($isPromise(maybePromise)) { maybePromise - .then(() => { + .$then(() => { this.emit("drain"); cb(null); }) - .catch(cb); + .$then(undefined, cb); return false; } else { cb(null); @@ -731,7 +731,7 @@ writeStreamPrototype._destroy = function (err, cb) { if (sink && sink !== true) { const end = sink.end(err); if ($isPromise(end)) { - end.then(() => cb(err), cb); + end.$then(() => cb(err), cb); return; } } @@ -790,7 +790,7 @@ Object.defineProperty(writeStreamPrototype, "pending", { function thenIfPromise(maybePromise: Promise | T, cb: any) { $assert(typeof cb === "function", "cb is not a function"); if ($isPromise(maybePromise)) { - maybePromise.then(() => cb(null), cb); + maybePromise.$then(() => cb(null), cb); } else { process.nextTick(cb, null); } diff --git a/src/js/internal/net/isIP.ts b/src/js/internal/net/isIP.ts index 93d38456404c..16b54d9f6bad 100644 --- a/src/js/internal/net/isIP.ts +++ b/src/js/internal/net/isIP.ts @@ -1,3 +1,5 @@ +const RegExpPrototypeExec = RegExp.prototype.exec; + const v4Seg = "(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])"; const v4Str = `(?:${v4Seg}\\.){3}${v4Seg}`; var IPv4Reg: RegExp | undefined; @@ -6,22 +8,27 @@ const v6Seg = "(?:[0-9a-fA-F]{1,4})"; var IPv6Reg: RegExp | undefined; function isIPv4(s) { - return (IPv4Reg ??= new RegExp(`^${v4Str}$`)).test(s); + return RegExpPrototypeExec.$call((IPv4Reg ??= new RegExp(`^${v4Str}$`)), s) !== null; } function isIPv6(s) { - return (IPv6Reg ??= new RegExp( - "^(?:" + - `(?:${v6Seg}:){7}(?:${v6Seg}|:)|` + - `(?:${v6Seg}:){6}(?:${v4Str}|:${v6Seg}|:)|` + - `(?:${v6Seg}:){5}(?::${v4Str}|(?::${v6Seg}){1,2}|:)|` + - `(?:${v6Seg}:){4}(?:(?::${v6Seg}){0,1}:${v4Str}|(?::${v6Seg}){1,3}|:)|` + - `(?:${v6Seg}:){3}(?:(?::${v6Seg}){0,2}:${v4Str}|(?::${v6Seg}){1,4}|:)|` + - `(?:${v6Seg}:){2}(?:(?::${v6Seg}){0,3}:${v4Str}|(?::${v6Seg}){1,5}|:)|` + - `(?:${v6Seg}:){1}(?:(?::${v6Seg}){0,4}:${v4Str}|(?::${v6Seg}){1,6}|:)|` + - `(?::(?:(?::${v6Seg}){0,5}:${v4Str}|(?::${v6Seg}){1,7}|:))` + - ")(?:%[0-9a-zA-Z-.:]{1,})?$", - )).test(s); + return ( + RegExpPrototypeExec.$call( + (IPv6Reg ??= new RegExp( + "^(?:" + + `(?:${v6Seg}:){7}(?:${v6Seg}|:)|` + + `(?:${v6Seg}:){6}(?:${v4Str}|:${v6Seg}|:)|` + + `(?:${v6Seg}:){5}(?::${v4Str}|(?::${v6Seg}){1,2}|:)|` + + `(?:${v6Seg}:){4}(?:(?::${v6Seg}){0,1}:${v4Str}|(?::${v6Seg}){1,3}|:)|` + + `(?:${v6Seg}:){3}(?:(?::${v6Seg}){0,2}:${v4Str}|(?::${v6Seg}){1,4}|:)|` + + `(?:${v6Seg}:){2}(?:(?::${v6Seg}){0,3}:${v4Str}|(?::${v6Seg}){1,5}|:)|` + + `(?:${v6Seg}:){1}(?:(?::${v6Seg}){0,4}:${v4Str}|(?::${v6Seg}){1,6}|:)|` + + `(?::(?:(?::${v6Seg}){0,5}:${v4Str}|(?::${v6Seg}){1,7}|:))` + + ")(?:%[0-9a-zA-Z-.:]{1,})?$", + )), + s, + ) !== null + ); } function isIP(s) { diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index 29eee5b7b3f2..c11564d9a62e 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -564,7 +564,7 @@ function spawnSync(file, args, options) { // normalizeSpawnargs has already prepended argv0 to the spawnargs array // Bun.spawn() expects cmd[0] to be the command to run, and argv0 to replace the first arg when running the command, // so we have to set argv0 to spawnargs[0] and cmd[0] to file - cmd: [options.file, ...Array.prototype.slice.$call(options.args, 1)], + cmd: spawnCommand(options.file, options.args), // The normalized env (kBunEnv) is built from the live process.env when // options.env is not given, so runtime mutations of process.env reach // the child like in Node.js. @@ -797,7 +797,12 @@ function fork(modulePath, args = [], options) { } } - args = [...execArgv, modulePath, ...args]; + const forkArgs = ArrayPrototypeSlice.$call(execArgv); + ArrayPrototypePush.$call(forkArgs, modulePath); + for (let i = 0; i < args.length; i++) { + ArrayPrototypePush.$call(forkArgs, args[i]); + } + args = forkArgs; if (typeof options.stdio === "string") { options.stdio = stdioStringToArray(options.stdio, "ipc"); @@ -908,6 +913,14 @@ function normalizeExecArgs(command, options, callback) { }; } +// spawnargs carries argv0 at index 0 (normalizeSpawnArguments prepends it); +// Bun.spawn() wants the program in cmd[0] and takes argv0 separately. +function spawnCommand(file, spawnargs) { + const cmd = ArrayPrototypeSlice.$call(spawnargs); + cmd[0] = file; + return cmd; +} + const kBunEnv = Symbol("bunEnv"); function normalizeSpawnArguments(file, args, options) { validateString(file, "file"); @@ -985,7 +998,7 @@ function normalizeSpawnArguments(file, args, options) { // Handle shell if (shell) { validateArgumentNullCheck(shell, "options.shell"); - const command = ArrayPrototypeJoin.$call([file, ...args], " "); + const command = args.length === 0 ? file : file + " " + ArrayPrototypeJoin.$call(args, " "); // Set the shell, switches, and commands. if (process.platform === "win32") { if (typeof shell === "string") file = shell; @@ -1044,7 +1057,8 @@ function normalizeSpawnArguments(file, args, options) { }); } - for (const key of envKeys) { + for (let i = 0; i < envKeys.length; i++) { + const key = envKeys[i]; const value = env[key]; if (value !== undefined) { validateArgumentNullCheck(key, `options.env['${key}']`); @@ -1410,7 +1424,7 @@ class ChildProcess extends EventEmitter { try { this.#handle = Bun.spawn({ - cmd: [file, ...Array.prototype.slice.$call(spawnargs, 1)], + cmd: spawnCommand(file, spawnargs), stdio: bunStdio, cwd: options.cwd || undefined, env: env, diff --git a/src/js/node/fs.promises.ts b/src/js/node/fs.promises.ts index 2c0bdf175caf..c93a9e55b170 100644 --- a/src/js/node/fs.promises.ts +++ b/src/js/node/fs.promises.ts @@ -248,9 +248,9 @@ async function* glob(pattern, options) { const exports = { access: asyncWrap(fs.access, "access"), - appendFile: async function (fileHandleOrFdOrPath, ...args) { + appendFile: async function (fileHandleOrFdOrPath, data, options) { fileHandleOrFdOrPath = fileHandleOrFdOrPath?.[kFd] ?? fileHandleOrFdOrPath; - return _appendFile(fileHandleOrFdOrPath, ...args); + return _appendFile(fileHandleOrFdOrPath, data, options); }, close: asyncWrap(fs.close, "close"), copyFile: asyncWrap(fs.copyFile, "copyFile"), @@ -304,23 +304,22 @@ const exports = { read: asyncWrap(fs.read, "read"), write: asyncWrap(fs.write, "write"), readdir: asyncWrap(fs.readdir, "readdir"), - readFile: async function (fileHandleOrFdOrPath, ...args) { + readFile: async function (fileHandleOrFdOrPath, options) { fileHandleOrFdOrPath = fileHandleOrFdOrPath?.[kFd] ?? fileHandleOrFdOrPath; - return _readFile(fileHandleOrFdOrPath, ...args); + return _readFile(fileHandleOrFdOrPath, options); }, - writeFile: async function (fileHandleOrFdOrPath, ...args: any[]) { + writeFile: async function (fileHandleOrFdOrPath, data, options) { fileHandleOrFdOrPath = fileHandleOrFdOrPath?.[kFd] ?? fileHandleOrFdOrPath; if ( - !$isTypedArrayView(args[0]) && - typeof args[0] !== "string" && - ($isCallable(args[0]?.[Symbol.iterator]) || $isCallable(args[0]?.[Symbol.asyncIterator])) + !$isTypedArrayView(data) && + typeof data !== "string" && + ($isCallable(data?.[Symbol.iterator]) || $isCallable(data?.[Symbol.asyncIterator])) ) { $debug("fs.promises.writeFile async iterator slow path!"); // Node accepts an arbitrary async iterator here - // @ts-expect-error - return writeFileAsyncIterator(fileHandleOrFdOrPath, ...args); + return writeFileAsyncIterator(fileHandleOrFdOrPath, data, options); } - return _writeFile(fileHandleOrFdOrPath, ...args); + return _writeFile(fileHandleOrFdOrPath, data, options); }, readlink: asyncWrap(fs.readlink, "readlink"), realpath: asyncWrap(fs.realpath, "realpath"), @@ -1603,7 +1602,7 @@ function flagTruncates(flag): boolean { return flag === "w" || flag === "w+" || flag === "wx" || flag === "wx+" || flag === "xw" || flag === "xw+"; } -async function writeFileAsyncIterator(fdOrPath, iterable, optionsOrEncoding, flag, mode) { +async function writeFileAsyncIterator(fdOrPath, iterable, optionsOrEncoding, flag?, mode?) { let encoding; let signal: AbortSignal | null = null; if (typeof optionsOrEncoding === "object") { diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index 29433463bbc4..9ee844e4f25e 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -62,7 +62,7 @@ var access = function access(path, mode, callback) { } callback = ensureCallback(callback); - fs.access(path, mode).then(callback, callback); + fs.access(path, mode).$then(callback, callback); }, appendFile = function appendFile(path, data, options, callback) { if (!$isCallable(callback)) { @@ -72,14 +72,14 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.appendFile(path, data, options).then(nullcallback(callback), callback); + fs.appendFile(path, data, options).$then(nullcallback(callback), callback); }, close = function close(fd, callback) { if ($isCallable(callback)) { callback = wrapFsCallback(callback); - fs.close(fd).then(() => callback(null), callback); + fs.close(fd).$then(() => callback(null), callback); } else if (callback === undefined) { - fs.close(fd).then(() => {}); + fs.close(fd).$then(() => {}); } else { callback = ensureCallback(callback); } @@ -92,7 +92,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); // route through promises.rm for the JS-side ERR_FS_EISDIR validation - require("node:fs/promises").rm(path, options).then(nullcallback(callback), callback); + require("node:fs/promises").rm(path, options).$then(nullcallback(callback), callback); }, rmdir = function rmdir(path, options, callback) { if ($isCallable(options)) { @@ -102,7 +102,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); // Node 26 removed `recursive` (DEP0147), but packages still pass it. Keep it working through `rm`. - (options?.recursive ? require("node:fs/promises").rm(path, options) : fs.rmdir(path, options)).then( + (options?.recursive ? require("node:fs/promises").rm(path, options) : fs.rmdir(path, options)).$then( nullcallback(callback), callback, ); @@ -115,13 +115,13 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.copyFile(src, dest, mode).then(nullcallback(callback), callback); + fs.copyFile(src, dest, mode).$then(nullcallback(callback), callback); }, exists = function exists(path, callback) { callback = ensureCallback(callback); try { - fs.exists.$apply(fs, [path]).then( + fs.exists.$apply(fs, [path]).$then( existed => callback(existed), _ => callback(false), ); @@ -132,22 +132,22 @@ var access = function access(path, mode, callback) { chown = function chown(path, uid, gid, callback) { callback = ensureCallback(callback); - fs.chown(path, uid, gid).then(nullcallback(callback), callback); + fs.chown(path, uid, gid).$then(nullcallback(callback), callback); }, chmod = function chmod(path, mode, callback) { callback = ensureCallback(callback); - fs.chmod(path, mode).then(nullcallback(callback), callback); + fs.chmod(path, mode).$then(nullcallback(callback), callback); }, fchmod = function fchmod(fd, mode, callback) { callback = ensureCallback(callback); - fs.fchmod(fd, mode).then(nullcallback(callback), callback); + fs.fchmod(fd, mode).$then(nullcallback(callback), callback); }, fchown = function fchown(fd, uid, gid, callback) { callback = ensureCallback(callback); - fs.fchown(fd, uid, gid).then(nullcallback(callback), callback); + fs.fchown(fd, uid, gid).$then(nullcallback(callback), callback); }, fstat = function fstat(fd, options, callback) { if ($isCallable(options)) { @@ -156,14 +156,14 @@ var access = function access(path, mode, callback) { } callback = wrapFsCallback(callback); - fs.fstat(fd, options).then(function (stats) { + fs.fstat(fd, options).$then(function (stats) { callback(null, stats); }, callback); }, fsync = function fsync(fd, callback) { callback = ensureCallback(callback); - fs.fsync(fd).then(nullcallback(callback), callback); + fs.fsync(fd).$then(nullcallback(callback), callback); }, ftruncate = function ftruncate(fd, len = 0, callback) { if ($isCallable(len)) { @@ -173,30 +173,30 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.ftruncate(fd, len).then(nullcallback(callback), callback); + fs.ftruncate(fd, len).$then(nullcallback(callback), callback); }, futimes = function futimes(fd, atime, mtime, callback) { callback = ensureCallback(callback); - fs.futimes(fd, atime, mtime).then(nullcallback(callback), callback); + fs.futimes(fd, atime, mtime).$then(nullcallback(callback), callback); }, lchmod = constants.O_SYMLINK !== undefined ? function lchmod(path, mode, callback) { callback = ensureCallback(callback); - fs.lchmod(path, mode).then(nullcallback(callback), callback); + fs.lchmod(path, mode).$then(nullcallback(callback), callback); } : undefined, // lchmod is only available on macOS lchown = function lchown(path, uid, gid, callback) { callback = ensureCallback(callback); - fs.lchown(path, uid, gid).then(nullcallback(callback), callback); + fs.lchown(path, uid, gid).$then(nullcallback(callback), callback); }, link = function link(existingPath, newPath, callback) { callback = ensureCallback(callback); - fs.link(existingPath, newPath).then(nullcallback(callback), callback); + fs.link(existingPath, newPath).$then(nullcallback(callback), callback); }, mkdir = function mkdir(path, options, callback) { if ($isCallable(options)) { @@ -206,7 +206,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.mkdir(path, options).then(nullcallback(callback), callback); + fs.mkdir(path, options).$then(nullcallback(callback), callback); }, mkdtemp = function mkdtemp(prefix, options, callback) { if ($isCallable(options)) { @@ -216,7 +216,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.mkdtemp(prefix, options).then(function (folder) { + fs.mkdtemp(prefix, options).$then(function (folder) { callback(null, folder); }, callback); }, @@ -232,14 +232,14 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.open(path, flags, mode).then(function (fd) { + fs.open(path, flags, mode).$then(function (fd) { callback(null, fd); }, callback); }, fdatasync = function fdatasync(fd, callback) { callback = ensureCallback(callback); - fs.fdatasync(fd).then(nullcallback(callback), callback); + fs.fdatasync(fd).$then(nullcallback(callback), callback); }, read = function read(fd, buffer, offsetOrOptions, length, position, callback) { // fd = getValidatedFd(fd); DEFERRED TO NATIVE @@ -281,7 +281,7 @@ var access = function access(path, mode, callback) { throw $ERR_INVALID_ARG_TYPE("callback", "function", callback); } callback = wrapFsCallback(callback); - fs.read(fd, buffer, offset, length, position).then( + fs.read(fd, buffer, offset, length, position).$then( bytesRead => void callback(null, bytesRead, buffer), err => callback(err), ); @@ -305,7 +305,7 @@ var access = function access(path, mode, callback) { } = offsetOrOptions ?? {}); } - fs.write(fd, buffer, offsetOrOptions, length, position).then(wrapper, callback); + fs.write(fd, buffer, offsetOrOptions, length, position).$then(wrapper, callback); return; } @@ -328,7 +328,7 @@ var access = function access(path, mode, callback) { callback = position; callback = ensureCallback(callback); - fs.write(fd, buffer, offsetOrOptions, length).then(wrapper, callback); + fs.write(fd, buffer, offsetOrOptions, length).$then(wrapper, callback); }, readdir = function readdir(path, options, callback) { if ($isCallable(options)) { @@ -338,7 +338,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.readdir(path, options).then(function (files) { + fs.readdir(path, options).$then(function (files) { callback(null, files); }, callback); }, @@ -346,7 +346,7 @@ var access = function access(path, mode, callback) { callback ||= options; callback = ensureCallback(callback); - fs.readFile(path, options).then(function (data) { + fs.readFile(path, options).$then(function (data) { callback(null, data); }, callback); }, @@ -354,7 +354,7 @@ var access = function access(path, mode, callback) { callback ||= options; callback = ensureCallback(callback); - fs.writeFile(path, data, options).then(nullcallback(callback), callback); + fs.writeFile(path, data, options).$then(nullcallback(callback), callback); }, readlink = function readlink(path, options, callback?) { if ($isCallable(options)) { @@ -364,14 +364,14 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.readlink(path, options).then(function (linkString) { + fs.readlink(path, options).$then(function (linkString) { callback(null, linkString); }, callback); }, rename = function rename(oldPath, newPath, callback) { callback = ensureCallback(callback); - fs.rename(oldPath, newPath).then(nullcallback(callback), callback); + fs.rename(oldPath, newPath).$then(nullcallback(callback), callback); }, lstat = function lstat(path, options, callback?) { if ($isCallable(options)) { @@ -381,7 +381,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.lstat(path, options).then(function (stats) { + fs.lstat(path, options).$then(function (stats) { callback(null, stats); }, callback); }, @@ -399,7 +399,7 @@ var access = function access(path, mode, callback) { return; } - fs.stat(path, options).then(function (stats) { + fs.stat(path, options).$then(function (stats) { callback(null, stats); }, callback); }, @@ -411,7 +411,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); - fs.statfs(path, options).then(function (stats) { + fs.statfs(path, options).$then(function (stats) { callback(null, stats); }, callback); }, @@ -425,7 +425,7 @@ var access = function access(path, mode, callback) { callback = wrapFsCallback(callback); } - fs.symlink(target, path, type).then(callback, callback); + fs.symlink(target, path, type).$then(callback, callback); }, truncate = function truncate(path, len, callback) { if (typeof path === "number") { @@ -442,22 +442,22 @@ var access = function access(path, mode, callback) { } callback = ensureCallback(callback); - fs.truncate(path, len).then(nullcallback(callback), callback); + fs.truncate(path, len).$then(nullcallback(callback), callback); }, unlink = function unlink(path, callback) { callback = ensureCallback(callback); - fs.unlink(path).then(nullcallback(callback), callback); + fs.unlink(path).$then(nullcallback(callback), callback); }, utimes = function utimes(path, atime, mtime, callback) { callback = ensureCallback(callback); - fs.utimes(path, atime, mtime).then(nullcallback(callback), callback); + fs.utimes(path, atime, mtime).$then(nullcallback(callback), callback); }, lutimes = function lutimes(path, atime, mtime, callback) { callback = ensureCallback(callback); - fs.lutimes(path, atime, mtime).then(nullcallback(callback), callback); + fs.lutimes(path, atime, mtime).$then(nullcallback(callback), callback); }, accessSync = fs.accessSync.bind(fs), appendFileSync = fs.appendFileSync.bind(fs), @@ -628,7 +628,7 @@ var access = function access(path, mode, callback) { // Invoke the callback from process.nextTick so an exception thrown by it // surfaces as an uncaught exception instead of rejecting this internal // promise chain (same convention as glob() below). - fs.stat(path).then( + fs.stat(path).$then( onOpendirStatFulfilled.bind(null, callback, path, result), onOpendirStatRejected.bind(null, callback, path), ); @@ -807,7 +807,7 @@ const realpath: typeof import("node:fs").realpath = } callback = ensureCallback(callback); - fs.realpath(p, options, false).then(function (resolvedPath) { + fs.realpath(p, options, false).$then(function (resolvedPath) { callback(null, resolvedPath); }, callback); } as typeof import("node:fs").realpath) @@ -958,7 +958,7 @@ realpath.native = function realpath(p, options, callback) { callback = ensureCallback(callback); - fs.realpathNative(p, options).then(function (resolvedPath) { + fs.realpathNative(p, options).$then(function (resolvedPath) { callback(null, resolvedPath); }, callback); }; @@ -1002,7 +1002,7 @@ function cp(src, dest, options, callback) { dest = getValidatedFsPath(dest, "dest"); callback = guardCallback(callback); - require("node:fs/promises").cp(src, dest, options).then(callOnceWithNull.bind(null, callback), callback); + require("node:fs/promises").cp(src, dest, options).$then(callOnceWithNull.bind(null, callback), callback); } function _toUnixTimestamp(time: any, name = "time") { @@ -1130,7 +1130,7 @@ class Dir { const prev = this.#pendingOp; let p; if (prev) { - p = prev.then(run, run); + p = prev.$then(run, run); } else { try { const r = run(); @@ -1142,7 +1142,7 @@ class Dir { this.#pendingCount++; this.#pendingOp = p; const done = this.#opDone.bind(this); - p.then(done, done); + p.$then(done, done); return p; } @@ -1163,7 +1163,7 @@ class Dir { validateFunction(cb, "callback"); cb = guardCallback(cb); // node's callback overload returns undefined (like close(cb) above) - this.read().then(callOnceWithNullThen.bind(null, cb), cb); + this.read().$then(callOnceWithNullThen.bind(null, cb), cb); return; } @@ -1184,7 +1184,7 @@ class Dir { encoding: this.#options?.encoding, recursive: this.#options?.recursive, }) - .then(this.#onReaddir.bind(this)); + .$then(this.#onReaddir.bind(this)); } #onReaddir(entries) { @@ -1204,7 +1204,7 @@ class Dir { if (!$isUndefinedOrNull(cb)) { validateFunction(cb, "callback"); cb = guardCallback(cb); - this.close().then(callOnceWithNull.bind(null, cb), cb); + this.close().$then(callOnceWithNull.bind(null, cb), cb); return; } return this.#enqueue(this.#closeOp.bind(this)); @@ -1261,7 +1261,7 @@ function glob(pattern: string | string[], options, callback) { // the callback surfaces as an uncaught exception instead of rejecting the // internal promise chain (and is never routed back into `callback` as an // error), matching Node.js. - Array.fromAsync(lazyGlob().glob(pattern, options ?? kEmptyObject)).then( + Array.fromAsync(lazyGlob().glob(pattern, options ?? kEmptyObject)).$then( nextTickWithNullThen.bind(null, callback), nextTickWith.bind(null, callback), ); diff --git a/test/js/bun/resolve/require-primordials.test.ts b/test/js/bun/resolve/require-primordials.test.ts new file mode 100644 index 000000000000..65195de9fa95 --- /dev/null +++ b/test/js/bun/resolve/require-primordials.test.ts @@ -0,0 +1,41 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// The CommonJS loader decides "built-in module?" and "native addon?" from the +// resolved id. Those checks must not run through the user-visible +// String.prototype methods: a patched endsWith() would hand a plain .cjs file +// to dlopen, and a patched startsWith() would break require("node:*"). +test("require() dispatch ignores patched String.prototype.startsWith/endsWith", async () => { + using dir = tempDir("require-primordials", { + "m.cjs": 'module.exports = "cjs-ok";', + "fixture.js": ` + const origEndsWith = String.prototype.endsWith; + const origStartsWith = String.prototype.startsWith; + String.prototype.endsWith = function (search, ...rest) { + return search === ".node" ? true : origEndsWith.call(this, search, ...rest); + }; + String.prototype.startsWith = function (search, ...rest) { + return search === "node:" ? false : origStartsWith.call(this, search, ...rest); + }; + let out; + try { + out = { cjs: require("./m.cjs"), builtin: typeof require("node:fs").readFileSync, bare: typeof require("fs").statSync }; + } catch (err) { + out = { error: err.message }; + } + String.prototype.endsWith = origEndsWith; + String.prototype.startsWith = origStartsWith; + console.log(JSON.stringify(out)); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ cjs: "cjs-ok", builtin: "function", bare: "function" }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/node/child_process/child_process-primordials.test.ts b/test/js/node/child_process/child_process-primordials.test.ts new file mode 100644 index 000000000000..f3f80b96331a --- /dev/null +++ b/test/js/node/child_process/child_process-primordials.test.ts @@ -0,0 +1,75 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// The argv handed to a child must not depend on user-patched Array built-ins +// (a polyfill with an off-by-one, an instrumentation wrapper). Each API runs +// once before the patch so every module it needs is loaded, like a polyfill +// installed after startup. The grandchild prints the argv it received. +test("spawnSync, spawn, and fork pass argv through with patched Array.prototype.slice and iterator", async () => { + using dir = tempDir("child-process-primordials", { + "fixture.js": ` + const { spawnSync, spawn, fork } = require("child_process"); + // "bun -e" leaves process.argv as [execPath, ...args]. + const printArgv = "console.log(JSON.stringify(process.argv.slice(1)))"; + const forkOptions = { stdio: ["pipe", "pipe", "pipe", "ipc"] }; + const closed = child => new Promise(resolve => child.on("close", resolve)); + const collect = (child, chunks) => child.stdout.on("data", d => chunks.push(d)); + + (async () => { + spawnSync(process.execPath, ["--version"]); + await Promise.all([closed(spawn(process.execPath, ["--version"])), closed(fork("forked.js", [], forkOptions))]); + + const origSlice = Array.prototype.slice; + const origIterator = Array.prototype[Symbol.iterator]; + // slice() duplicates the last element, the iterator skips the first one. + Array.prototype.slice = function (...args) { + const result = origSlice.apply(this, args); + if (result.length) result.push(result[result.length - 1]); + return result; + }; + Array.prototype[Symbol.iterator] = function () { + const it = origIterator.call(this); + it.next(); + return it; + }; + + const sync = spawnSync(process.execPath, ["-e", printArgv, "intended", "last"]); + const child = spawn(process.execPath, ["-e", printArgv, "intended", "last"]); + const forked = fork("forked.js", ["intended", "last"], forkOptions); + + Array.prototype.slice = origSlice; + Array.prototype[Symbol.iterator] = origIterator; + + const childOut = []; + const forkOut = []; + collect(child, childOut); + collect(forked, forkOut); + await closed(child); + await closed(forked); + console.log( + JSON.stringify({ + sync: String(sync.stdout).trim(), + async: childOut.join("").trim(), + fork: forkOut.join("").trim(), + }), + ); + })(); + `, + // fork() puts the script before the arguments: [execPath, script, ...args]. + "forked.js": "console.log(JSON.stringify(process.argv.slice(2)))", + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + sync: '["intended","last"]', + async: '["intended","last"]', + fork: '["intended","last"]', + }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/node/fs/fs-primordials.test.ts b/test/js/node/fs/fs-primordials.test.ts new file mode 100644 index 000000000000..43b992a94d60 --- /dev/null +++ b/test/js/node/fs/fs-primordials.test.ts @@ -0,0 +1,126 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// node:fs must keep working when user code replaces a built-in with a +// wrapper that changes results (a buggy polyfill, an instrumentation shim). +// Each case loads the modules first and patches the built-in afterwards, the +// way a polyfill installed after startup does. + +async function runFixture(dir: string, file: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), file], + env: bunEnv, + cwd: dir, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +test.concurrent("fs.cpSync copies every entry when Array.prototype[Symbol.iterator] is patched", async () => { + using dir = tempDir("fs-cpsync-iterator", { + "fixture.js": ` + const fs = require("fs"); + const path = require("path"); + fs.mkdirSync("d/e", { recursive: true }); + for (const f of ["d/1.txt", "d/2.txt", "d/e/3.txt"]) fs.writeFileSync(f, f); + // Load internal/fs/cp-sync before the patch. + fs.cpSync("d/1.txt", "warm.txt"); + + const orig = Array.prototype[Symbol.iterator]; + // A broken iterator polyfill that skips the first element. + Array.prototype[Symbol.iterator] = function () { + const it = orig.call(this); + it.next(); + return it; + }; + fs.cpSync("d", "d2", { recursive: true }); + Array.prototype[Symbol.iterator] = orig; + + const copied = fs.readdirSync("d2", { recursive: true }).map(p => p.replaceAll(path.sep, "/")).sort(); + console.log(JSON.stringify(copied)); + `, + }); + const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); + expect(stdout).toBe('["1.txt","2.txt","e","e/3.txt"]\n'); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); +}); + +test.concurrent( + "fs.promises.readFile keeps its encoding when Array.prototype[Symbol.iterator] is patched", + async () => { + using dir = tempDir("fs-readfile-iterator", { + "in.txt": "hello", + "fixture.js": ` + const fs = require("fs"); + const orig = Array.prototype[Symbol.iterator]; + const skipsFirst = function () { + const it = orig.call(this); + it.next(); + return it; + }; + // The patch is live only while the call reads its arguments. + function withPatch(fn) { + Array.prototype[Symbol.iterator] = skipsFirst; + try { + return fn(); + } finally { + Array.prototype[Symbol.iterator] = orig; + } + } + (async () => { + const text = await withPatch(() => fs.promises.readFile("in.txt", "utf8")); + await withPatch(() => fs.promises.writeFile("out.txt", "written", "utf8")); + await withPatch(() => fs.promises.appendFile("out.txt", "+more", "utf8")); + console.log(typeof text, text, fs.readFileSync("out.txt", "utf8")); + })(); + `, + }); + const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); + expect(stdout).toBe("string hello written+more\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }, +); + +test.concurrent("fs streams keep their file descriptor when Promise.prototype.then is patched", async () => { + using dir = tempDir("fs-streams-then", { + "in.txt": "hello", + "fixture.js": ` + const fs = require("fs"); + const orig = Promise.prototype.then; + const seen = []; + // An instrumentation shim that changes resolved numbers by one. + Promise.prototype.then = function (onFulfilled, onRejected) { + return orig.call( + this, + value => { + seen.push(typeof value === "number" ? value : typeof value); + return onFulfilled(typeof value === "number" ? value + 1 : value); + }, + onRejected, + ); + }; + + const w = fs.createWriteStream("out.txt"); + w.on("error", err => console.log("write stream error:", err.code)); + w.end("abcdef", () => { + const r = fs.createReadStream("in.txt"); + let acc = ""; + r.on("data", d => (acc += d)); + r.on("error", err => console.log("read stream error:", err.code)); + r.on("close", () => { + Promise.prototype.then = orig; + console.log(JSON.stringify({ out: fs.readFileSync("out.txt", "utf8"), read: acc, seen })); + }); + }); + `, + }); + const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); + // Stock bun 1.4.0 printed EBADF errors for both streams and seen: [7, 8]: + // the descriptors passed through the patched then() and came back off by one. + expect(stdout).toBe(JSON.stringify({ out: "abcdef", read: "hello", seen: [] }) + "\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); +}); diff --git a/test/js/node/net/net-isip-primordials.test.ts b/test/js/node/net/net-isip-primordials.test.ts new file mode 100644 index 000000000000..65d9c6df030e --- /dev/null +++ b/test/js/node/net/net-isip-primordials.test.ts @@ -0,0 +1,44 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// net.isIP() (also the "is this already an address?" check in dns.lookup) must +// not consult the user-visible RegExp.prototype.test: a patched test() would +// make dns.lookup hand the hostname back as the resolved address. +test("net.isIP and dns.lookup ignore a patched RegExp.prototype.test", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const net = require("net"); + const dns = require("dns"); + const origTest = RegExp.prototype.test; + RegExp.prototype.test = function (s) { + return !origTest.call(this, s); + }; + const classified = { + v4: net.isIP("127.0.0.1"), + v6: net.isIP("::1"), + host: net.isIP("localhost"), + isIPv4: net.isIPv4("10.0.0.1"), + isIPv6: net.isIPv6("fe80::1%lo0"), + }; + dns.lookup("localhost", { family: 4 }, (err, address, family) => { + RegExp.prototype.test = origTest; + console.log(JSON.stringify({ classified, err: err?.code ?? null, address, family })); + }); + `, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const result = JSON.parse(stdout); + expect(result.classified).toEqual({ v4: 4, v6: 6, host: 0, isIPv4: true, isIPv6: true }); + // The resolver answers, not the patched classifier echoing the hostname back. + expect(result.err).toBeNull(); + expect(result.address).not.toBe("localhost"); + expect(result.family).toBe(4); + expect(exitCode).toBe(0); +}); From f29c15e74b14f6dd07a06510034d1142160330c5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 05:44:25 +0000 Subject: [PATCH 2/9] Windows process.env proxy: stop reading Array and String built-ins live The ownKeys trap returned envMapList.slice() through the live Array.prototype.slice, so the patched slice in the new child_process test made the trap return a duplicate key and JSC rejected the enumeration ("ownKeys trap result must not contain any duplicate names") from inside normalizeSpawnArguments. The other traps read toUpperCase, includes, some, findIndex, splice, push, and Reflect.getOwnPropertyDescriptor live as well. Capture them when the proxy is created and iterate by index. Also use the captured slice on the deferred spawn error path, pre-create the cpSync destination in the fs test so macOS runs the JS walker instead of clonefile, and report stdout, stderr, and the exit code together in the child_process test. --- src/js/builtins/ProcessObjectInternals.ts | 62 ++++++++++++------- src/js/node/child_process.ts | 2 +- .../child_process-primordials.test.ts | 15 +++-- test/js/node/fs/fs-primordials.test.ts | 3 + 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 37501f664d0b..0f0505f560a5 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -464,24 +464,38 @@ export function windowsEnv( coerceForWrite, resetForDelete, ) { - (internalEnv as any)[Bun.inspect.custom] = () => { - let o = {}; - for (let k of envMapList) { - o[k] = internalEnv[k.toUpperCase()]; - } - return o; - }; + // Every process.env access on Windows runs these traps, including the env + // that child_process hands to a child, so they must not read built-ins that + // user code can have patched in the meantime. + const ArrayPrototypeSlice = Array.prototype.slice; + const ArrayPrototypeIncludes = Array.prototype.includes; + const ArrayPrototypePush = Array.prototype.push; + const ArrayPrototypeSplice = Array.prototype.splice; + const StringPrototypeToUpperCase = String.prototype.toUpperCase; + const ReflectGetOwnPropertyDescriptor = Reflect.getOwnPropertyDescriptor; + + // Index of the entry naming the same variable as the uppercase key k, or -1. + function indexOfEnvKey(k: string) { + for (let i = 0; i < envMapList.length; i++) { + if (StringPrototypeToUpperCase.$call(envMapList[i]) === k) return i; + } + return -1; + } - (internalEnv as any).toJSON = () => { + function toPlainObject() { // Mirror enumeration: original-case key names, case-insensitive values. // Spreading internalEnv directly would leak the canonical UPPERCASE // storage keys into JSON.stringify(process.env) and IPC env echoes. let o = {}; - for (let k of envMapList) { - o[k] = internalEnv[k.toUpperCase()]; + for (let i = 0; i < envMapList.length; i++) { + const k = envMapList[i]; + o[k] = internalEnv[StringPrototypeToUpperCase.$call(k)]; } return o; - }; + } + + (internalEnv as any)[Bun.inspect.custom] = toPlainObject; + (internalEnv as any).toJSON = toPlainObject; // Shared write path for the `set` and `defineProperty` traps. Plain // assignment (never Object.defineProperty on the target) keeps the @@ -493,8 +507,8 @@ export function windowsEnv( // Track the key for enumeration if it isn't already there. Don't gate on // `k in internalEnv`: the proxy accessors (HTTP_PROXY, ...) always exist // as DontEnum CustomAccessors even when the variable was never set. - if (!envMapList.includes(p) && !envMapList.some(x => x.toUpperCase() === k)) { - envMapList.push(p); + if (!ArrayPrototypeIncludes.$call(envMapList, p) && indexOfEnvKey(k) === -1) { + ArrayPrototypePush.$call(envMapList, p); } if (internalEnv[k] !== coerced) { editWindowsEnvVar(k, coerced); @@ -510,7 +524,7 @@ export function windowsEnv( } // Env-var lookup is case-insensitive on Windows: the canonical // uppercase key wins when the variable exists. - const k = p.toUpperCase(); + const k = StringPrototypeToUpperCase.$call(p); if (k in internalEnv) { return internalEnv[k]; } @@ -525,7 +539,7 @@ export function windowsEnv( if (typeof p === "symbol" || typeof value === "symbol") { throw new TypeError("Cannot convert a Symbol value to a string"); } - const k = p.toUpperCase(); + const k = StringPrototypeToUpperCase.$call(p); // Node silently ignores assignments to an empty variable name // (https://github.com/nodejs/node/issues/32920). if (k === "") { @@ -539,7 +553,7 @@ export function windowsEnv( // Case-insensitive env-var query first, then ordinary lookup so own // as-is properties and Object.prototype methods answer `in` like node // (`'hasOwnProperty' in process.env` is true on all platforms). - if (typeof p === "string" && p.toUpperCase() in internalEnv) { + if (typeof p === "string" && StringPrototypeToUpperCase.$call(p) in internalEnv) { return true; } return p in internalEnv; @@ -549,10 +563,10 @@ export function windowsEnv( if (typeof p === "symbol") { return true; } - const k = String(p).toUpperCase(); - const i = envMapList.findIndex(x => x.toUpperCase() === k); + const k = StringPrototypeToUpperCase.$call(p); + const i = indexOfEnvKey(k); if (i !== -1) { - envMapList.splice(i, 1); + ArrayPrototypeSplice.$call(envMapList, i, 1); } editWindowsEnvVar(k, null); // internalEnv is a plain object here so `delete internalEnv[k]` never @@ -588,7 +602,7 @@ export function windowsEnv( if (typeof attributes.value === "symbol") { throw new TypeError("Cannot convert a Symbol value to a string"); } - const k = p.toUpperCase(); + const k = StringPrototypeToUpperCase.$call(p); // Node silently ignores an empty variable name, like the set trap. if (k === "") { return true; @@ -600,15 +614,15 @@ export function windowsEnv( }, getOwnPropertyDescriptor(target, p) { if (typeof p === "string") { - const desc = Reflect.getOwnPropertyDescriptor(target, p.toUpperCase()); + const desc = ReflectGetOwnPropertyDescriptor(target, StringPrototypeToUpperCase.$call(p)); if (desc) return desc; } // Own as-is properties (toJSON, Bun.inspect.custom symbol). - return Reflect.getOwnPropertyDescriptor(target, p); + return ReflectGetOwnPropertyDescriptor(target, p); }, ownKeys() { - // .slice() because paranoia that there is a way to call this without the engine cloning it for us - return envMapList.slice(); + // A copy: the engine validates the result and the caller may keep it. + return ArrayPrototypeSlice.$call(envMapList); }, }); } diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index c11564d9a62e..0dced31d4e20 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -1499,7 +1499,7 @@ class ChildProcess extends EventEmitter { ) { this.#handle = null; ex.syscall = "spawn " + this.spawnfile; - ex.spawnargs = Array.prototype.slice.$call(this.spawnargs, 1); + ex.spawnargs = ArrayPrototypeSlice.$call(this.spawnargs, 1); process.nextTick(() => { this.emit("error", ex); this.emit("close", (ex as SystemError).errno ?? -1); diff --git a/test/js/node/child_process/child_process-primordials.test.ts b/test/js/node/child_process/child_process-primordials.test.ts index f3f80b96331a..c333b015c922 100644 --- a/test/js/node/child_process/child_process-primordials.test.ts +++ b/test/js/node/child_process/child_process-primordials.test.ts @@ -65,11 +65,14 @@ test("spawnSync, spawn, and fork pass argv through with patched Array.prototype. stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).toBe(""); - expect(JSON.parse(stdout)).toEqual({ - sync: '["intended","last"]', - async: '["intended","last"]', - fork: '["intended","last"]', + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: + JSON.stringify({ + sync: '["intended","last"]', + async: '["intended","last"]', + fork: '["intended","last"]', + }) + "\n", + stderr: "", + exitCode: 0, }); - expect(exitCode).toBe(0); }); diff --git a/test/js/node/fs/fs-primordials.test.ts b/test/js/node/fs/fs-primordials.test.ts index 43b992a94d60..a54c67003d7f 100644 --- a/test/js/node/fs/fs-primordials.test.ts +++ b/test/js/node/fs/fs-primordials.test.ts @@ -26,6 +26,9 @@ test.concurrent("fs.cpSync copies every entry when Array.prototype[Symbol.iterat for (const f of ["d/1.txt", "d/2.txt", "d/e/3.txt"]) fs.writeFileSync(f, f); // Load internal/fs/cp-sync before the patch. fs.cpSync("d/1.txt", "warm.txt"); + // An existing destination keeps macOS off the clonefile fast path, so + // the JS directory walker runs on every platform. + fs.mkdirSync("d2"); const orig = Array.prototype[Symbol.iterator]; // A broken iterator polyfill that skips the first element. From 710d6d3f310569e19a2966c23a596c35cebc83bd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 05:47:39 +0000 Subject: [PATCH 3/9] Shorten the comments at the captured built-in sites --- src/js/builtins/CommonJS.ts | 3 +-- src/js/builtins/ProcessObjectInternals.ts | 4 +--- src/js/node/child_process.ts | 10 +--------- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/js/builtins/CommonJS.ts b/src/js/builtins/CommonJS.ts index 37576335dc85..b8beca2fb5a4 100644 --- a/src/js/builtins/CommonJS.ts +++ b/src/js/builtins/CommonJS.ts @@ -17,8 +17,7 @@ $overriddenName = "require"; $visibility = "Private"; export function overridableRequire(this: JSCommonJSModule, originalId: string, options?: { paths?: string[] }) { const id = $resolveSync(originalId, this.filename, false, false, options ? options.paths : undefined, this, options); - // $stringSubstring / $stringIndexOfInternal are the engine's own String - // methods; a user-patched String.prototype must not pick the loader. + // $stringSubstring / $stringIndexOfInternal: user code can replace String.prototype.*. if ($stringSubstring.$call(id, 0, 5) === "node:") { if (id !== originalId) { // A terrible special case where Node.js allows non-prefixed built-ins to diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 0f0505f560a5..d91595483993 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -464,9 +464,7 @@ export function windowsEnv( coerceForWrite, resetForDelete, ) { - // Every process.env access on Windows runs these traps, including the env - // that child_process hands to a child, so they must not read built-ins that - // user code can have patched in the meantime. + // Captured once: user code can replace these on the prototypes later. const ArrayPrototypeSlice = Array.prototype.slice; const ArrayPrototypeIncludes = Array.prototype.includes; const ArrayPrototypePush = Array.prototype.push; diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index 0dced31d4e20..ec8edc0a2fc8 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -561,9 +561,6 @@ function spawnSync(file, args, options) { exitedDueToMaxBuffer, pid, } = Bun.spawnSync({ - // normalizeSpawnargs has already prepended argv0 to the spawnargs array - // Bun.spawn() expects cmd[0] to be the command to run, and argv0 to replace the first arg when running the command, - // so we have to set argv0 to spawnargs[0] and cmd[0] to file cmd: spawnCommand(options.file, options.args), // The normalized env (kBunEnv) is built from the live process.env when // options.env is not given, so runtime mutations of process.env reach @@ -913,8 +910,7 @@ function normalizeExecArgs(command, options, callback) { }; } -// spawnargs carries argv0 at index 0 (normalizeSpawnArguments prepends it); -// Bun.spawn() wants the program in cmd[0] and takes argv0 separately. +// spawnargs[0] is argv0 (normalizeSpawnArguments prepends it); Bun.spawn() takes argv0 separately and wants the program in cmd[0]. function spawnCommand(file, spawnargs) { const cmd = ArrayPrototypeSlice.$call(spawnargs); cmd[0] = file; @@ -1418,10 +1414,6 @@ class ChildProcess extends EventEmitter { validateArray(options.args, "options.args"); spawnargs = this.spawnargs = options.args; } - // normalizeSpawnargs has already prepended argv0 to the spawnargs array - // Bun.spawn() expects cmd[0] to be the command to run, and argv0 to replace the first arg when running the command, - // so we have to set argv0 to spawnargs[0] and cmd[0] to file - try { this.#handle = Bun.spawn({ cmd: spawnCommand(file, spawnargs), From e3c6b02ce1e1a917dbd6767dc147fcb2786e2f6b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 06:16:52 +0000 Subject: [PATCH 4/9] child_process primordials test: attach both close listeners before awaiting The fixture awaited the spawned child's close event and only then attached the listener for the forked child. When the forked child closed first, that listener missed the event, nothing kept the event loop alive, and the fixture exited 0 with no output (10 of 150 runs under load). --- test/js/node/child_process/child_process-primordials.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/js/node/child_process/child_process-primordials.test.ts b/test/js/node/child_process/child_process-primordials.test.ts index c333b015c922..0f13233b1ba8 100644 --- a/test/js/node/child_process/child_process-primordials.test.ts +++ b/test/js/node/child_process/child_process-primordials.test.ts @@ -44,8 +44,9 @@ test("spawnSync, spawn, and fork pass argv through with patched Array.prototype. const forkOut = []; collect(child, childOut); collect(forked, forkOut); - await closed(child); - await closed(forked); + // Both "close" listeners go on before the first await: a child that + // closes while the other one is awaited must not be missed. + await Promise.all([closed(child), closed(forked)]); console.log( JSON.stringify({ sync: String(sync.stdout).trim(), From 7019ed3fe229c82fbc43b84cfd8fc7ffe4c8a6f3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 06:34:09 +0000 Subject: [PATCH 5/9] ci: retrigger From a41df89aa716a0b555d78363a9ed15d30f30f37c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 06:44:17 +0000 Subject: [PATCH 6/9] process.stdout: resolve the FileSink flush with the private then --- src/js/builtins/ProcessObjectInternals.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index d91595483993..41583c5c307f 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -93,7 +93,7 @@ export function getStdioWriteStream( if (sink && sink !== true) { const result = sink.flush(); if ($isPromise(result)) { - result.then( + result.$then( () => cb(null), err => cb(err), ); From e8ea8a8313f933f02699826e060ec7299a71e1f3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 24 Aug 2026 07:24:47 +0000 Subject: [PATCH 7/9] fs streams: resolve the FileHandle close with the private then --- src/js/internal/fs/streams.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 89ea82ae9d31..4bea44faa07b 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -84,7 +84,10 @@ const fileHandleStreamFs = (fh: FileHandle) => ({ function streamFileHandleClose(this: FileHandle, fd: FD, cb: (err?: any) => void) { $assert(this[kFd] == fd, "fd mismatch"); this[kUnref](); - this.close().then(() => cb(), cb); + const closed = this.close(); + // Only a FileHandle whose close() the user replaced can hand back a thenable. + if ($isPromise(closed)) closed.$then(() => cb(), cb); + else closed.then(() => cb(), cb); } function getValidatedPath(p: any) { From f2284379078858ed107b914acced949cc7604725 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:26:04 +0000 Subject: [PATCH 8/9] fs, child_process: accept thenables and reject a non-iterable execArgv again; fix the FileHandle sync fast path fs.rm, fs.rmdir({ recursive }), fs.cp and fs.glob call a function that user code can replace (fs.promises.rm, fs.promises.cp, Array.fromAsync). The result can be a plain thenable, which has no private then, so the earlier switch to .$then made these throw synchronously. Use the private then only when the result is a real promise. fork() copied execArgv with slice, which turns a non-iterable value into an empty list and starts the child without the flags. Node throws a TypeError there. Keep the spread for a non-array execArgv. internal/fs/streams compared fh.sync with FileHandle.prototype.fsync, which does not exist, so every FileHandle stream took the fallback that resolves through the public then. With flush: true and a then that never calls its handlers, the stream never closed. Compare with FileHandle.prototype.sync. --- src/js/internal/fs/streams.ts | 4 +- src/js/node/child_process.ts | 3 +- src/js/node/fs.ts | 16 +++-- .../child_process-primordials.test.ts | 29 +++++++++ test/js/node/fs/fs-primordials.test.ts | 62 +++++++++++++++++++ 5 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 4bea44faa07b..0722c5045a9c 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -45,7 +45,7 @@ const kFs = Symbol("kFs"); const { read: fileHandlePrototypeRead, write: fileHandlePrototypeWrite, - fsync: fileHandlePrototypeFsync, + sync: fileHandlePrototypeSync, writev: fileHandlePrototypeWritev, } = FileHandle.prototype; @@ -73,7 +73,7 @@ const fileHandleStreamFs = (fh: FileHandle) => ({ }, writev: fh.writev === fileHandlePrototypeWritev ? writev : undefined, fsync: - fh.sync === fileHandlePrototypeFsync + fh.sync === fileHandlePrototypeSync ? fsync : function (fd, cb) { return fh.sync().then(() => cb(), cb); diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index ec8edc0a2fc8..9203a3e59bc6 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -794,7 +794,8 @@ function fork(modulePath, args = [], options) { } } - const forkArgs = ArrayPrototypeSlice.$call(execArgv); + // A non-array execArgv keeps the spread: a non-iterable value must still throw, like Node. + const forkArgs = $isJSArray(execArgv) ? ArrayPrototypeSlice.$call(execArgv) : [...execArgv]; ArrayPrototypePush.$call(forkArgs, modulePath); for (let i = 0; i < args.length; i++) { ArrayPrototypePush.$call(forkArgs, args[i]); diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index 9ee844e4f25e..9c29468b0b1b 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -46,6 +46,12 @@ function ensureCallback(callback) { // // function () { callback(null); } // +// For a callee that user code can replace (fs.promises.rm, Array.fromAsync): the result may be a plain thenable. +function thenMaybeNative(result, onFulfilled, onRejected) { + if ($isPromise(result)) result.$then(onFulfilled, onRejected); + else result.then(onFulfilled, onRejected); +} + function nullcallback(callback) { return FunctionPrototypeBind.$call(callback, undefined, null); } @@ -92,7 +98,7 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); // route through promises.rm for the JS-side ERR_FS_EISDIR validation - require("node:fs/promises").rm(path, options).$then(nullcallback(callback), callback); + thenMaybeNative(require("node:fs/promises").rm(path, options), nullcallback(callback), callback); }, rmdir = function rmdir(path, options, callback) { if ($isCallable(options)) { @@ -102,7 +108,8 @@ var access = function access(path, mode, callback) { callback = ensureCallback(callback); // Node 26 removed `recursive` (DEP0147), but packages still pass it. Keep it working through `rm`. - (options?.recursive ? require("node:fs/promises").rm(path, options) : fs.rmdir(path, options)).$then( + thenMaybeNative( + options?.recursive ? require("node:fs/promises").rm(path, options) : fs.rmdir(path, options), nullcallback(callback), callback, ); @@ -1002,7 +1009,7 @@ function cp(src, dest, options, callback) { dest = getValidatedFsPath(dest, "dest"); callback = guardCallback(callback); - require("node:fs/promises").cp(src, dest, options).$then(callOnceWithNull.bind(null, callback), callback); + thenMaybeNative(require("node:fs/promises").cp(src, dest, options), callOnceWithNull.bind(null, callback), callback); } function _toUnixTimestamp(time: any, name = "time") { @@ -1261,7 +1268,8 @@ function glob(pattern: string | string[], options, callback) { // the callback surfaces as an uncaught exception instead of rejecting the // internal promise chain (and is never routed back into `callback` as an // error), matching Node.js. - Array.fromAsync(lazyGlob().glob(pattern, options ?? kEmptyObject)).$then( + thenMaybeNative( + Array.fromAsync(lazyGlob().glob(pattern, options ?? kEmptyObject)), nextTickWithNullThen.bind(null, callback), nextTickWith.bind(null, callback), ); diff --git a/test/js/node/child_process/child_process-primordials.test.ts b/test/js/node/child_process/child_process-primordials.test.ts index 0f13233b1ba8..e5db61b75a67 100644 --- a/test/js/node/child_process/child_process-primordials.test.ts +++ b/test/js/node/child_process/child_process-primordials.test.ts @@ -77,3 +77,32 @@ test("spawnSync, spawn, and fork pass argv through with patched Array.prototype. exitCode: 0, }); }); + +// The argv copy must not swallow what Node reports: a non-iterable execArgv is a TypeError, not an empty list. +test("fork() throws for a non-iterable execArgv", async () => { + using dir = tempDir("child-process-fork-execargv", { + "child.js": "process.exit(0)", + "fixture.js": ` + const { fork } = require("child_process"); + let result; + try { + const child = fork("child.js", [], { execArgv: { inspect: true } }); + child.on("close", () => console.log(JSON.stringify({ result: "spawned" }))); + } catch (err) { + console.log(JSON.stringify({ result: "threw", name: err.name })); + } + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ result: "threw", name: "TypeError" }) + "\n", + stderr: "", + exitCode: 0, + }); +}); diff --git a/test/js/node/fs/fs-primordials.test.ts b/test/js/node/fs/fs-primordials.test.ts index a54c67003d7f..401085b678ca 100644 --- a/test/js/node/fs/fs-primordials.test.ts +++ b/test/js/node/fs/fs-primordials.test.ts @@ -127,3 +127,65 @@ test.concurrent("fs streams keep their file descriptor when Promise.prototype.th expect(stderr).toBe(""); expect(exitCode).toBe(0); }); + +test.concurrent( + "a FileHandle write stream with flush: true closes when Promise.prototype.then is patched", + async () => { + using dir = tempDir("fs-filehandle-flush-then", { + "fixture.js": ` + const fs = require("fs"); + (async () => { + const fh = await fs.promises.open("out.txt", "w"); + const orig = Promise.prototype.then; + // A replacement that never calls its handlers. + Promise.prototype.then = function () { + return 42; + }; + const w = fh.createWriteStream({ flush: true }); + w.on("close", () => { + Promise.prototype.then = orig; + console.log(JSON.stringify({ closed: true, content: fs.readFileSync("out.txt", "utf8") })); + }); + w.end("data"); + })(); + `, + }); + const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ closed: true, content: "data" }) + "\n", + stderr: "", + exitCode: 0, + }); + }, +); + +// fs.rm and fs.cp call whatever fs.promises.rm / fs.promises.cp currently are, so a +// test double that returns a plain thenable must keep working. +test.concurrent("fs.rm and fs.cp accept a thenable from a replaced fs.promises.rm / cp", async () => { + using dir = tempDir("fs-live-callee-thenable", { + "fixture.js": ` + const fs = require("fs"); + const fsp = require("fs/promises"); + const calls = []; + const thenable = name => ({ + then(onFulfilled) { + calls.push(name); + onFulfilled(); + }, + }); + fsp.rm = () => thenable("rm"); + fsp.cp = () => thenable("cp"); + fs.rm("a.txt", rmErr => { + fs.cp("a.txt", "b.txt", cpErr => { + console.log(JSON.stringify({ rmErr: rmErr ?? null, cpErr: cpErr ?? null, calls })); + }); + }); + `, + }); + const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ rmErr: null, cpErr: null, calls: ["rm", "cp"] }) + "\n", + stderr: "", + exitCode: 0, + }); +}); From ce12a478b2f41973b1fc1f72c5072775a1136bc0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:56:30 +0000 Subject: [PATCH 9/9] child_process: iterate stdio by index after spawn; define cmd[0] as an own property ChildProcess#spawn walked this.stdio with for...of after Bun.spawn() had started the child. A replaced Array.prototype[Symbol.iterator] that throws made spawn() and fork() throw to the caller while the child kept running, and one that skips an element left a stdio socket without its ref(). Iterate by index. spawnCommand() assigned cmd[0] = file. With no args the copy is empty, so that assignment consults the prototype chain and a setter for index 0 on Array.prototype received the file while cmd stayed empty ("cmd must not be empty"). The array literal this replaced defined own properties. Define the element directly. The new test runs spawnSync, spawn and fork under an iterator that throws. The fs test file now uses one assertion form throughout. --- src/js/node/child_process.ts | 8 ++- .../child_process-primordials.test.ts | 60 +++++++++++++++++++ test/js/node/fs/fs-primordials.test.ts | 20 ++++--- 3 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index 9203a3e59bc6..a09e07031acf 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -914,7 +914,8 @@ function normalizeExecArgs(command, options, callback) { // spawnargs[0] is argv0 (normalizeSpawnArguments prepends it); Bun.spawn() takes argv0 separately and wants the program in cmd[0]. function spawnCommand(file, spawnargs) { const cmd = ArrayPrototypeSlice.$call(spawnargs); - cmd[0] = file; + // Not cmd[0] = file: with no own index 0 (empty args) that assignment reaches a setter on Array.prototype. + $putByValDirect(cmd, 0, file); return cmd; } @@ -1476,8 +1477,9 @@ class ChildProcess extends EventEmitter { } if (hasSocketsToEagerlyLoad) { - for (let item of this.stdio) { - item?.ref?.(); + const stdio = this.stdio; + for (let i = 0; i < stdio.length; i++) { + stdio[i]?.ref?.(); } } } catch (ex) { diff --git a/test/js/node/child_process/child_process-primordials.test.ts b/test/js/node/child_process/child_process-primordials.test.ts index e5db61b75a67..49a66220e60c 100644 --- a/test/js/node/child_process/child_process-primordials.test.ts +++ b/test/js/node/child_process/child_process-primordials.test.ts @@ -106,3 +106,63 @@ test("fork() throws for a non-iterable execArgv", async () => { exitCode: 0, }); }); + +// A throwing iterator is the strict form of the case above: no array may be iterated at all on these +// paths. spawn() used to for...of its stdio after the child had started, so the caller got the throw +// while the child kept running. +test("spawnSync, spawn, and fork never call Array.prototype[Symbol.iterator]", async () => { + using dir = tempDir("child-process-iterator-throws", { + "fixture.js": ` + const { spawnSync, spawn, fork } = require("child_process"); + const printArgv = "console.log(JSON.stringify(process.argv.slice(1)))"; + const forkOptions = { stdio: ["pipe", "pipe", "pipe", "ipc"] }; + const closed = child => new Promise(resolve => child.on("close", resolve)); + const collect = (child, chunks) => child.stdout.on("data", d => chunks.push(d)); + + (async () => { + spawnSync(process.execPath, ["--version"]); + await Promise.all([closed(spawn(process.execPath, ["--version"])), closed(fork("forked.js", [], forkOptions))]); + + const origIterator = Array.prototype[Symbol.iterator]; + Array.prototype[Symbol.iterator] = function () { + throw new Error("Array.prototype[Symbol.iterator] was called"); + }; + let sync, child, forked; + try { + sync = spawnSync(process.execPath, ["-e", printArgv, "a", "b"]); + child = spawn(process.execPath, ["-e", printArgv, "a", "b"]); + forked = fork("forked.js", ["a", "b"], forkOptions); + } finally { + // Restore before anything prints, so a failure reports cleanly. + Array.prototype[Symbol.iterator] = origIterator; + } + + const childOut = []; + const forkOut = []; + collect(child, childOut); + collect(forked, forkOut); + await Promise.all([closed(child), closed(forked)]); + console.log( + JSON.stringify({ + sync: String(sync.stdout).trim(), + async: childOut.join("").trim(), + fork: forkOut.join("").trim(), + }), + ); + })(); + `, + "forked.js": "console.log(JSON.stringify(process.argv.slice(2)))", + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ sync: '["a","b"]', async: '["a","b"]', fork: '["a","b"]' }) + "\n", + stderr: "", + exitCode: 0, + }); +}); diff --git a/test/js/node/fs/fs-primordials.test.ts b/test/js/node/fs/fs-primordials.test.ts index 401085b678ca..6afa09fa809e 100644 --- a/test/js/node/fs/fs-primordials.test.ts +++ b/test/js/node/fs/fs-primordials.test.ts @@ -45,9 +45,11 @@ test.concurrent("fs.cpSync copies every entry when Array.prototype[Symbol.iterat `, }); const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); - expect(stdout).toBe('["1.txt","2.txt","e","e/3.txt"]\n'); - expect(stderr).toBe(""); - expect(exitCode).toBe(0); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: '["1.txt","2.txt","e","e/3.txt"]\n', + stderr: "", + exitCode: 0, + }); }); test.concurrent( @@ -81,9 +83,7 @@ test.concurrent( `, }); const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); - expect(stdout).toBe("string hello written+more\n"); - expect(stderr).toBe(""); - expect(exitCode).toBe(0); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "string hello written+more\n", stderr: "", exitCode: 0 }); }, ); @@ -123,9 +123,11 @@ test.concurrent("fs streams keep their file descriptor when Promise.prototype.th const { stdout, stderr, exitCode } = await runFixture(String(dir), "fixture.js"); // Stock bun 1.4.0 printed EBADF errors for both streams and seen: [7, 8]: // the descriptors passed through the patched then() and came back off by one. - expect(stdout).toBe(JSON.stringify({ out: "abcdef", read: "hello", seen: [] }) + "\n"); - expect(stderr).toBe(""); - expect(exitCode).toBe(0); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ out: "abcdef", read: "hello", seen: [] }) + "\n", + stderr: "", + exitCode: 0, + }); }); test.concurrent(