From 15603cd97b9043bb4fe060ae6f67c22328c91a8b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:58:22 +0000 Subject: [PATCH 1/2] node:fs: accept Buffer and Uint8Array paths in cp, cpSync, and promises.cp The node cp port (#31830) runs node:path checks on the validated src and dest before the native copy. getValidatedFsPath passes a Buffer through unchanged, so path.dirname threw ERR_INVALID_ARG_TYPE. bun 1.3.14 took Buffer paths in all three functions. Decode the bytes to a string once, after validation, in a helper shared by the three entry points. --- src/js/internal/fs/cp-sync.ts | 19 +++++++++++- src/js/node/fs.promises.ts | 7 ++--- src/js/node/fs.ts | 14 ++++----- test/js/node/fs/cp.test.ts | 57 +++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/js/internal/fs/cp-sync.ts b/src/js/internal/fs/cp-sync.ts index be4de692fcbb..39e0f5e08251 100644 --- a/src/js/internal/fs/cp-sync.ts +++ b/src/js/internal/fs/cp-sync.ts @@ -2,7 +2,13 @@ // Also hosts the option validation and SystemError construction shared with // internal/fs/cp (async) and the fs.cpSync/fs.cp/fs.promises.cp dispatchers, // ported from node lib/internal/fs/utils.js and lib/internal/errors.js. -const { validateObject, validateBoolean, validateFunction, validateInteger } = require("internal/validators"); +const { + validateObject, + validateBoolean, + validateFunction, + validateInteger, + getValidatedFsPath, +} = require("internal/validators"); const { chmodSync, copyFileSync, @@ -22,6 +28,7 @@ const { EEXIST, EISDIR, EINVAL, ENOTDIR } = $processBindingConstants.os.errno; const ArrayPrototypeEvery = Array.prototype.every; const ArrayPrototypeFilter = Array.prototype.filter; +const BufferPrototypeToString = Buffer.prototype.toString; const StringPrototypeSplit = String.prototype.split; // COPYFILE_EXCL | COPYFILE_FICLONE | COPYFILE_FICLONE_FORCE @@ -142,6 +149,15 @@ function validateCpOptions(options) { return options; } +// node's getValidatedPath passes a Buffer or Uint8Array through unchanged, and +// node's cpSync hands it to C++ bindings that take raw bytes. The checks and +// the walker here run on node:path, which takes only strings, so the bytes are +// decoded as UTF-8 once, up front. +function getValidatedCpPath(p, propName) { + p = getValidatedFsPath(p, propName); + return typeof p === "string" ? p : BufferPrototypeToString.$call(p); +} + function areIdentical(srcStat, destStat) { return destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev; } @@ -510,6 +526,7 @@ function copyLink(resolvedSrc, dest) { export default { cpSyncFn, validateCpOptions, + getValidatedCpPath, tryNativeFastPathSync, errno: { EEXIST, EISDIR, EINVAL, ENOTDIR }, fsCpDirToNonDirError, diff --git a/src/js/node/fs.promises.ts b/src/js/node/fs.promises.ts index 2c0bdf175caf..9da3673f24c6 100644 --- a/src/js/node/fs.promises.ts +++ b/src/js/node/fs.promises.ts @@ -177,11 +177,10 @@ function watch( // and on MacOS, simple cases of recursive directory trees can be done in a single `clonefile()` // using filter and other options uses a lazily loaded js fallback ported from node.js async function cp(src, dest, options) { - const { validateCpOptions } = require("internal/fs/cp-sync"); - const { getValidatedFsPath } = require("internal/validators"); + const { validateCpOptions, getValidatedCpPath } = require("internal/fs/cp-sync"); options = validateCpOptions(options); - src = getValidatedFsPath(src, "src"); - dest = getValidatedFsPath(dest, "dest"); + src = getValidatedCpPath(src, "src"); + dest = getValidatedCpPath(dest, "dest"); const { filter, dereference, preserveTimestamps, verbatimSymlinks, mode, errorOnExist, force, recursive } = options; if (!filter && !dereference && !preserveTimestamps && !verbatimSymlinks && !mode && !errorOnExist && force) { const { ok, checked } = await require("internal/fs/cp").tryNativeFastPath(src, dest, options); diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index 29433463bbc4..4dd6b1f3ef83 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -968,11 +968,10 @@ realpathSync.native = fs.realpathNativeSync.bind(fs); // and on MacOS, simple cases of recursive directory trees can be done in a single `clonefile()` // using filter and other options uses a lazily loaded js fallback ported from node.js function cpSync(src, dest, options) { - const { cpSyncFn, validateCpOptions, tryNativeFastPathSync } = require("internal/fs/cp-sync"); - const { getValidatedFsPath } = require("internal/validators"); + const { cpSyncFn, validateCpOptions, getValidatedCpPath, tryNativeFastPathSync } = require("internal/fs/cp-sync"); options = validateCpOptions(options); - src = getValidatedFsPath(src, "src"); - dest = getValidatedFsPath(dest, "dest"); + src = getValidatedCpPath(src, "src"); + dest = getValidatedCpPath(dest, "dest"); const { filter, dereference, preserveTimestamps, verbatimSymlinks, mode, errorOnExist, force, recursive } = options; if (!filter && !dereference && !preserveTimestamps && !verbatimSymlinks && !mode && !errorOnExist && force) { const { ok, checked } = tryNativeFastPathSync(src, dest, options); @@ -995,11 +994,10 @@ function cp(src, dest, options, callback) { } // node's callback form throws synchronously on invalid options/paths - const { validateCpOptions } = require("internal/fs/cp-sync"); - const { getValidatedFsPath } = require("internal/validators"); + const { validateCpOptions, getValidatedCpPath } = require("internal/fs/cp-sync"); options = validateCpOptions(options); - src = getValidatedFsPath(src, "src"); - dest = getValidatedFsPath(dest, "dest"); + src = getValidatedCpPath(src, "src"); + dest = getValidatedCpPath(dest, "dest"); callback = guardCallback(callback); require("node:fs/promises").cp(src, dest, options).then(callOnceWithNull.bind(null, callback), callback); diff --git a/test/js/node/fs/cp.test.ts b/test/js/node/fs/cp.test.ts index c1960ed51689..d852d40321a1 100644 --- a/test/js/node/fs/cp.test.ts +++ b/test/js/node/fs/cp.test.ts @@ -37,6 +37,51 @@ for (const [name, copy] of impls) { expect(fs.readFileSync(basename + "/to.txt", "utf8")).toBe("a"); }); + test("Buffer and Uint8Array paths", async () => { + await using basename = tempDir("cp", { + "from/a.txt": "a", + "from/dir/b.txt": "b", + }); + const from = basename + "/from"; + + await copy(Buffer.from(from + "/a.txt"), basename + "/buf-src.txt"); + await copy(from + "/a.txt", Buffer.from(basename + "/buf-dest.txt")); + await copy(new Uint8Array(Buffer.from(from + "/a.txt")), basename + "/u8-src.txt"); + await copy(Buffer.from(from), Buffer.from(basename + "/buf-dir"), { recursive: true }); + + expect({ + bufSrc: fs.readFileSync(basename + "/buf-src.txt", "utf8"), + bufDest: fs.readFileSync(basename + "/buf-dest.txt", "utf8"), + u8Src: fs.readFileSync(basename + "/u8-src.txt", "utf8"), + bufDir: fs.readFileSync(basename + "/buf-dir/dir/b.txt", "utf8"), + }).toEqual({ bufSrc: "a", bufDest: "a", u8Src: "a", bufDir: "b" }); + }); + + test("Buffer paths with a filter - the filter receives string paths", async () => { + await using basename = tempDir("cp", { + "from/a.txt": "a", + "from/b.txt": "b", + }); + const from = join(basename, "from"); + const result = join(basename, "result"); + const seen: [string, string][] = []; + + await copy(Buffer.from(from), Buffer.from(result), { + recursive: true, + filter: (src: string, dest: string) => { + seen.push([src, dest]); + return !src.endsWith("b.txt"); + }, + }); + + expect(seen.sort((a, b) => a[0].localeCompare(b[0]))).toEqual([ + [from, result], + [join(from, "a.txt"), join(result, "a.txt")], + [join(from, "b.txt"), join(result, "b.txt")], + ]); + expect(fs.readdirSync(result)).toEqual(["a.txt"]); + }); + test("refuse to copy directory with 'recursive: false'", async () => { await using basename = tempDir("cp", { "from/a.txt": "a", @@ -430,6 +475,18 @@ test("cp with missing callback throws", () => { }).toThrow(/"cb"/); }); +test("cp callback form accepts Buffer paths", async () => { + await using basename = tempDir("cp", { + "a.txt": "a", + }); + + const { promise, resolve, reject } = Promise.withResolvers(); + fs.cp(Buffer.from(basename + "/a.txt"), Buffer.from(basename + "/b.txt"), err => (err ? reject(err) : resolve())); + await promise; + + expect(fs.readFileSync(basename + "/b.txt", "utf8")).toBe("a"); +}); + // On Windows, _copySingleFileSync's reparse-point branch opens a handle to the // source symlink to resolve its target via GetFinalPathNameByHandleW. Previously // that handle was never closed, leaking one OS handle per symlink copied. Over a From 234f1f4aaaf8958d56c2056614dea1ac6da8fe5d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:00:04 +0000 Subject: [PATCH 2/2] node:fs: keep Buffer paths as bytes through cp, cpSync, and promises.cp The node cp port (#31830) runs node:path checks and a JS walker on the validated src and dest before the native copy. getValidatedFsPath passes a Buffer through unchanged, so path.dirname threw ERR_INVALID_ARG_TYPE. bun 1.3.14 took Buffer paths in all three functions. A Buffer path now stays bytes all the way to the syscall. The checks and the walker run their dirname/join/resolve on a latin1 view of the bytes (one byte per code unit, and node:path inspects only ASCII) and turn the result back into bytes. Entry names and link targets under a Buffer path are read as bytes too. The filter and the error objects get the Buffers they were given. --- src/js/internal/fs/cp-sync.ts | 122 ++++++++++++++++++++++++---------- src/js/internal/fs/cp.ts | 50 ++++++++------ src/js/node/fs.promises.ts | 7 +- src/js/node/fs.ts | 14 ++-- test/js/node/fs/cp.test.ts | 50 +++++++++++--- 5 files changed, 171 insertions(+), 72 deletions(-) diff --git a/src/js/internal/fs/cp-sync.ts b/src/js/internal/fs/cp-sync.ts index 39e0f5e08251..3800f5d8f1fa 100644 --- a/src/js/internal/fs/cp-sync.ts +++ b/src/js/internal/fs/cp-sync.ts @@ -2,13 +2,7 @@ // Also hosts the option validation and SystemError construction shared with // internal/fs/cp (async) and the fs.cpSync/fs.cp/fs.promises.cp dispatchers, // ported from node lib/internal/fs/utils.js and lib/internal/errors.js. -const { - validateObject, - validateBoolean, - validateFunction, - validateInteger, - getValidatedFsPath, -} = require("internal/validators"); +const { validateObject, validateBoolean, validateFunction, validateInteger } = require("internal/validators"); const { chmodSync, copyFileSync, @@ -28,7 +22,7 @@ const { EEXIST, EISDIR, EINVAL, ENOTDIR } = $processBindingConstants.os.errno; const ArrayPrototypeEvery = Array.prototype.every; const ArrayPrototypeFilter = Array.prototype.filter; -const BufferPrototypeToString = Buffer.prototype.toString; +const BufferPrototypeLatin1Slice = Buffer.prototype.latin1Slice; const StringPrototypeSplit = String.prototype.split; // COPYFILE_EXCL | COPYFILE_FICLONE | COPYFILE_FICLONE_FORCE @@ -149,25 +143,81 @@ function validateCpOptions(options) { return options; } -// node's getValidatedPath passes a Buffer or Uint8Array through unchanged, and -// node's cpSync hands it to C++ bindings that take raw bytes. The checks and -// the walker here run on node:path, which takes only strings, so the bytes are -// decoded as UTF-8 once, up front. -function getValidatedCpPath(p, propName) { - p = getValidatedFsPath(p, propName); - return typeof p === "string" ? p : BufferPrototypeToString.$call(p); -} - function areIdentical(srcStat, destStat) { return destStat.ino && destStat.dev && destStat.ino === srcStat.ino && destStat.dev === srcStat.dev; } +// node:path inspects only ASCII, so a Buffer path is viewed as latin1 (one byte per code unit) for its arithmetic. +const isBufferPath = p => typeof p !== "string"; + +function latin1View(p) { + // A string meets a Buffer path through its UTF-8 bytes. + return BufferPrototypeLatin1Slice.$call(typeof p === "string" ? Buffer.from(p) : p); +} + +function fromLatin1View(view) { + return Buffer.from(view, "latin1"); +} + +// resolve() would fall back to process.cwd() as UTF-16; pass its view instead. +function cwdView() { + return latin1View(process.cwd()); +} + +const kBufferEncoding = { encoding: "buffer" }; + +// readdir/readlink options that return bytes for a Buffer path. +function encodingFor(p) { + return isBufferPath(p) ? kBufferEncoding : undefined; +} + +// dirname(p), in the form of p. +function parentPath(p) { + return isBufferPath(p) ? fromLatin1View(dirname(latin1View(p))) : dirname(p); +} + +// join(dir, name), as bytes when either is bytes. +function joinDirEntry(dir, name) { + if (!isBufferPath(dir) && !isBufferPath(name)) return join(dir, name); + return fromLatin1View(join(latin1View(dir), latin1View(name))); +} + +// The target for the copy of the link at linkPath, from what readlink returned. +function copiedLinkTarget(linkPath, target, verbatim) { + if (!isBufferPath(linkPath) && !isBufferPath(target)) { + return verbatim || isAbsolute(target) ? target : resolve(dirname(linkPath), target); + } + const targetView = latin1View(target); + if (verbatim || isAbsolute(targetView)) return target; + return fromLatin1View(resolve(cwdView(), dirname(latin1View(linkPath)), targetView)); +} + +// resolve(dirname()) of src and dest for checkParentPaths; destParentPath is the form that goes to stat. +function resolvedParents(src, dest) { + if (!isBufferPath(src) && !isBufferPath(dest)) { + const destParent = resolve(dirname(dest)); + return { srcParent: resolve(dirname(src)), destParent, destParentPath: destParent }; + } + const cwd = cwdView(); + const destParent = resolve(cwd, dirname(latin1View(dest))); + return { + srcParent: resolve(cwd, dirname(latin1View(src))), + destParent, + destParentPath: fromLatin1View(destParent), + }; +} + const normalizePathToArray = path => ArrayPrototypeFilter.$call(StringPrototypeSplit.$call(resolve(path), sep), Boolean); // Return true if dest is a subdir of src, otherwise false. // It only checks the path strings. function isSrcSubdir(src, dest) { + if (isBufferPath(src) || isBufferPath(dest)) { + const cwd = cwdView(); + src = resolve(cwd, latin1View(src)); + dest = resolve(cwd, latin1View(dest)); + } const srcArr = normalizePathToArray(src); const destArr = normalizePathToArray(dest); return ArrayPrototypeEvery.$call(srcArr, (cur, i) => destArr[i] === cur); @@ -241,12 +291,11 @@ function getStatsSync(src, dest, opts) { } function checkParentPathsSync(src, srcStat, dest) { - const srcParent = resolve(dirname(src)); - const destParent = resolve(dirname(dest)); + const { srcParent, destParent, destParentPath } = resolvedParents(src, dest); if (destParent === srcParent || destParent === parse(destParent).root) return; let destStat; try { - destStat = statSync(destParent, { bigint: true }); + destStat = statSync(destParentPath, { bigint: true }); } catch (err: any) { if (err.code === "ENOENT") return; throw err; @@ -260,7 +309,7 @@ function checkParentPathsSync(src, srcStat, dest) { code: "EINVAL", }); } - return checkParentPathsSync(src, srcStat, destParent); + return checkParentPathsSync(src, srcStat, destParentPath); } // The native recursive copy (a single clonefile() on macOS) copies symlinks @@ -283,7 +332,7 @@ function treeContainsOnlyFilesAndDirsSync(root) { for (let i = 0; i < entries.length; i++) { const entry = entries[i]; if (entry.isDirectory()) { - stack.push(join(dir, entry.name)); + stack.push(joinDirEntry(dir, entry.name)); } else if (!entry.isFile()) { return false; } @@ -334,7 +383,7 @@ function cpSyncFn(src, dest, opts, checked?) { } function checkParentDir(destStat, src, dest, opts) { - const destParent = dirname(dest); + const destParent = parentPath(dest); if (!existsSync(destParent)) mkdirSync(destParent, { recursive: true }); return getStats(destStat, src, dest, opts); } @@ -458,26 +507,23 @@ function mkDirAndCopy(srcMode, src, dest, opts) { } function copyDir(src, dest, opts) { - for (const dirent of readdirSync(src, { withFileTypes: true })) { - const { name } = dirent; - const srcItem = join(src, name); - const destItem = join(dest, name); + // Entry names under a Buffer directory are bytes too. + for (const name of readdirSync(src, encodingFor(src))) { + const srcItem = joinDirEntry(src, name); + const destItem = joinDirEntry(dest, name); const { destStat, skipped } = checkPathsSync(srcItem, destItem, opts); if (!skipped) getStats(destStat, srcItem, destItem, opts); } } function onLink(destStat, src, dest, opts) { - let resolvedSrc = readlinkSync(src); - if (!opts.verbatimSymlinks && !isAbsolute(resolvedSrc)) { - resolvedSrc = resolve(dirname(src), resolvedSrc); - } + const resolvedSrc = copiedLinkTarget(src, readlinkSync(src, encodingFor(src)), opts.verbatimSymlinks); if (!destStat) { return symlinkSync(resolvedSrc, dest); } let resolvedDest; try { - resolvedDest = readlinkSync(dest); + resolvedDest = readlinkSync(dest, encodingFor(dest)); } catch (err: any) { // Dest exists and is a regular file or directory, // Windows may throw UNKNOWN error. If dest already exists, @@ -487,9 +533,7 @@ function onLink(destStat, src, dest, opts) { } throw err; } - if (!isAbsolute(resolvedDest)) { - resolvedDest = resolve(dirname(dest), resolvedDest); - } + resolvedDest = copiedLinkTarget(dest, resolvedDest, false); let srcIsDir = false; try { srcIsDir = statSync(src).isDirectory(); @@ -526,8 +570,14 @@ function copyLink(resolvedSrc, dest) { export default { cpSyncFn, validateCpOptions, - getValidatedCpPath, tryNativeFastPathSync, + isBufferPath, + kBufferEncoding, + encodingFor, + parentPath, + joinDirEntry, + copiedLinkTarget, + resolvedParents, errno: { EEXIST, EISDIR, EINVAL, ENOTDIR }, fsCpDirToNonDirError, fsCpEExistError, diff --git a/src/js/internal/fs/cp.ts b/src/js/internal/fs/cp.ts index f7db620f191e..898c49f4393f 100644 --- a/src/js/internal/fs/cp.ts +++ b/src/js/internal/fs/cp.ts @@ -12,6 +12,13 @@ const { fsEisdirError, areIdentical, isSrcSubdir, + isBufferPath, + kBufferEncoding, + encodingFor, + parentPath, + joinDirEntry, + copiedLinkTarget, + resolvedParents, } = require("internal/fs/cp-sync"); const { @@ -27,7 +34,7 @@ const { unlink, utimes, } = require("node:fs/promises"); -const { dirname, isAbsolute, join, parse, resolve } = require("node:path"); +const { parse } = require("node:path"); const PromisePrototypeThen = $Promise.prototype.$then; const PromiseReject = Promise.$reject; @@ -95,14 +102,13 @@ function getStats(src, dest, opts) { // checks the src and dest inodes. It starts from the deepest // parent and stops once it reaches the src parent or the root path. async function checkParentPaths(src, srcStat, dest) { - const srcParent = resolve(dirname(src)); - const destParent = resolve(dirname(dest)); + const { srcParent, destParent, destParentPath } = resolvedParents(src, dest); if (destParent === srcParent || destParent === parse(destParent).root) { return; } let destStat; try { - destStat = await stat(destParent, { bigint: true }); + destStat = await stat(destParentPath, { bigint: true }); } catch (err: any) { if (err.code === "ENOENT") return; throw err; @@ -116,7 +122,7 @@ async function checkParentPaths(src, srcStat, dest) { code: "EINVAL", }); } - return checkParentPaths(src, srcStat, destParent); + return checkParentPaths(src, srcStat, destParentPath); } // The native recursive copy (a single clonefile() on macOS) copies symlinks @@ -139,7 +145,7 @@ async function treeContainsOnlyFilesAndDirs(root) { for (let i = 0; i < entries.length; i++) { const entry = entries[i]; if (entry.isDirectory()) { - stack.push(join(dir, entry.name)); + stack.push(joinDirEntry(dir, entry.name)); } else if (!entry.isFile()) { return false; } @@ -190,7 +196,7 @@ async function cpFn(src, dest, opts, checked?) { } async function checkParentDir(destStat, src, dest, opts) { - const destParent = dirname(dest); + const destParent = parentPath(dest); const dirExists = await pathExists(destParent); if (dirExists) return getStatsForCopy(destStat, src, dest, opts); await mkdir(destParent, { recursive: true }); @@ -335,27 +341,35 @@ async function mkDirAndCopy(srcMode, src, dest, opts) { } async function copyDir(src, dest, opts) { + if (isBufferPath(src)) { + // opendir() cannot return entry names as bytes (#27914), readdir() can. + for (const name of await readdir(src, kBufferEncoding)) { + await copyDirEntry(src, dest, name, opts); + } + return; + } const dir = await opendir(src); for await (const { name } of dir) { - const srcItem = join(src, name); - const destItem = join(dest, name); - const { destStat, skipped } = await checkPaths(srcItem, destItem, opts); - if (!skipped) await getStatsForCopy(destStat, srcItem, destItem, opts); + await copyDirEntry(src, dest, name, opts); } } +async function copyDirEntry(src, dest, name, opts) { + const srcItem = joinDirEntry(src, name); + const destItem = joinDirEntry(dest, name); + const { destStat, skipped } = await checkPaths(srcItem, destItem, opts); + if (!skipped) await getStatsForCopy(destStat, srcItem, destItem, opts); +} + async function onLink(destStat, src, dest, opts) { - let resolvedSrc = await readlink(src); - if (!opts.verbatimSymlinks && !isAbsolute(resolvedSrc)) { - resolvedSrc = resolve(dirname(src), resolvedSrc); - } + const resolvedSrc = copiedLinkTarget(src, await readlink(src, encodingFor(src)), opts.verbatimSymlinks); if (!destStat) { return symlink(resolvedSrc, dest); } let resolvedDest; try { - resolvedDest = await readlink(dest); + resolvedDest = await readlink(dest, encodingFor(dest)); } catch (err: any) { // Dest exists and is a regular file or directory, // Windows may throw UNKNOWN error. If dest already exists, @@ -365,9 +379,7 @@ async function onLink(destStat, src, dest, opts) { } throw err; } - if (!isAbsolute(resolvedDest)) { - resolvedDest = resolve(dirname(dest), resolvedDest); - } + resolvedDest = copiedLinkTarget(dest, resolvedDest, false); // stat(src) follows the link; a dangling src symlink throws ENOENT here, // same as before (both gated checks below only apply to directories). const srcStat = await stat(src); diff --git a/src/js/node/fs.promises.ts b/src/js/node/fs.promises.ts index 9da3673f24c6..2c0bdf175caf 100644 --- a/src/js/node/fs.promises.ts +++ b/src/js/node/fs.promises.ts @@ -177,10 +177,11 @@ function watch( // and on MacOS, simple cases of recursive directory trees can be done in a single `clonefile()` // using filter and other options uses a lazily loaded js fallback ported from node.js async function cp(src, dest, options) { - const { validateCpOptions, getValidatedCpPath } = require("internal/fs/cp-sync"); + const { validateCpOptions } = require("internal/fs/cp-sync"); + const { getValidatedFsPath } = require("internal/validators"); options = validateCpOptions(options); - src = getValidatedCpPath(src, "src"); - dest = getValidatedCpPath(dest, "dest"); + src = getValidatedFsPath(src, "src"); + dest = getValidatedFsPath(dest, "dest"); const { filter, dereference, preserveTimestamps, verbatimSymlinks, mode, errorOnExist, force, recursive } = options; if (!filter && !dereference && !preserveTimestamps && !verbatimSymlinks && !mode && !errorOnExist && force) { const { ok, checked } = await require("internal/fs/cp").tryNativeFastPath(src, dest, options); diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index 4dd6b1f3ef83..29433463bbc4 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -968,10 +968,11 @@ realpathSync.native = fs.realpathNativeSync.bind(fs); // and on MacOS, simple cases of recursive directory trees can be done in a single `clonefile()` // using filter and other options uses a lazily loaded js fallback ported from node.js function cpSync(src, dest, options) { - const { cpSyncFn, validateCpOptions, getValidatedCpPath, tryNativeFastPathSync } = require("internal/fs/cp-sync"); + const { cpSyncFn, validateCpOptions, tryNativeFastPathSync } = require("internal/fs/cp-sync"); + const { getValidatedFsPath } = require("internal/validators"); options = validateCpOptions(options); - src = getValidatedCpPath(src, "src"); - dest = getValidatedCpPath(dest, "dest"); + src = getValidatedFsPath(src, "src"); + dest = getValidatedFsPath(dest, "dest"); const { filter, dereference, preserveTimestamps, verbatimSymlinks, mode, errorOnExist, force, recursive } = options; if (!filter && !dereference && !preserveTimestamps && !verbatimSymlinks && !mode && !errorOnExist && force) { const { ok, checked } = tryNativeFastPathSync(src, dest, options); @@ -994,10 +995,11 @@ function cp(src, dest, options, callback) { } // node's callback form throws synchronously on invalid options/paths - const { validateCpOptions, getValidatedCpPath } = require("internal/fs/cp-sync"); + const { validateCpOptions } = require("internal/fs/cp-sync"); + const { getValidatedFsPath } = require("internal/validators"); options = validateCpOptions(options); - src = getValidatedCpPath(src, "src"); - dest = getValidatedCpPath(dest, "dest"); + src = getValidatedFsPath(src, "src"); + dest = getValidatedFsPath(dest, "dest"); callback = guardCallback(callback); require("node:fs/promises").cp(src, dest, options).then(callOnceWithNull.bind(null, callback), callback); diff --git a/test/js/node/fs/cp.test.ts b/test/js/node/fs/cp.test.ts index d852d40321a1..046d3a0729c3 100644 --- a/test/js/node/fs/cp.test.ts +++ b/test/js/node/fs/cp.test.ts @@ -57,31 +57,65 @@ for (const [name, copy] of impls) { }).toEqual({ bufSrc: "a", bufDest: "a", u8Src: "a", bufDir: "b" }); }); - test("Buffer paths with a filter - the filter receives string paths", async () => { + test("Buffer paths with a filter - the filter receives Buffer paths", async () => { await using basename = tempDir("cp", { "from/a.txt": "a", "from/b.txt": "b", }); const from = join(basename, "from"); const result = join(basename, "result"); - const seen: [string, string][] = []; + const seen: [Buffer, Buffer][] = []; await copy(Buffer.from(from), Buffer.from(result), { recursive: true, - filter: (src: string, dest: string) => { + filter: (src: any, dest: any) => { seen.push([src, dest]); - return !src.endsWith("b.txt"); + return !src.toString().endsWith("b.txt"); }, }); - expect(seen.sort((a, b) => a[0].localeCompare(b[0]))).toEqual([ - [from, result], - [join(from, "a.txt"), join(result, "a.txt")], - [join(from, "b.txt"), join(result, "b.txt")], + expect(seen.sort((a, b) => Buffer.compare(a[0], b[0]))).toEqual([ + [Buffer.from(from), Buffer.from(result)], + [Buffer.from(join(from, "a.txt")), Buffer.from(join(result, "a.txt"))], + [Buffer.from(join(from, "b.txt")), Buffer.from(join(result, "b.txt"))], ]); expect(fs.readdirSync(result)).toEqual(["a.txt"]); }); + // macOS rejects file names that are not valid UTF-8 (EILSEQ), and Windows + // names are UTF-16, so only Linux can hold the bytes this test needs. + test.skipIf(!isLinux)("Buffer paths are passed to the syscalls byte for byte", async () => { + await using basename = tempDir("cp", {}); + const bytes = (...parts: (string | Buffer)[]) => Buffer.concat(parts.map(p => Buffer.from(p))); + // "from\xff" and "caf\xe9" are not valid UTF-8; a UTF-8 decode turns them into U+FFFD. + const from = bytes(basename + "/from", Buffer.from([0xff])); + const entry = bytes("caf", Buffer.from([0xe9]), ".txt"); + const result = bytes(basename + "/result", Buffer.from([0xfe])); + fs.mkdirSync(from); + fs.mkdirSync(bytes(from, "/sub")); + fs.writeFileSync(bytes(from, "/", entry), "cafe"); + fs.writeFileSync(bytes(from, "/a.txt"), "a"); + fs.symlinkSync("../a.txt", bytes(from, "/sub/link")); + + // Single file: the native copy. + await copy(bytes(from, "/", entry), bytes(basename, "/", entry)); + // Directory: the walker, which reads the entry names as bytes and + // resolves the relative link target against the Buffer source tree. + await copy(from, result, { recursive: true }); + + expect({ + file: fs.readFileSync(bytes(basename, "/", entry), "utf8"), + entries: fs.readdirSync(result, { encoding: "buffer" }).sort(Buffer.compare), + content: fs.readFileSync(bytes(result, "/", entry), "utf8"), + link: fs.readlinkSync(bytes(result, "/sub/link"), { encoding: "buffer" }), + }).toEqual({ + file: "cafe", + entries: [Buffer.from("a.txt"), entry, Buffer.from("sub")].sort(Buffer.compare), + content: "cafe", + link: bytes(from, "/a.txt"), + }); + }); + test("refuse to copy directory with 'recursive: false'", async () => { await using basename = tempDir("cp", { "from/a.txt": "a",