fs: support Buffer paths in cp() and cpSync()

lib/internal/fs/cp/cp-sync.js

@@ -3,7 +3,9 @@
 // This file is a modified version of the fs-extra's copySync method.
 
 const fsBinding = internalBinding('fs');
-const { isSrcSubdir } = require('internal/fs/cp/cp');
+const { isSrcSubdir, joinPath } = require('internal/fs/cp/cp');
+const { Buffer } = require('buffer');
+const { isBuffer: BufferIsBuffer } = Buffer;
 const { codes: {
   ERR_FS_CP_EEXIST,
   ERR_FS_CP_EINVAL,
@@ -33,7 +35,6 @@ const {
 const {
   dirname,
   isAbsolute,
-  join,
   resolve,
 } = require('path');
 const { isPromise } = require('util/types');
@@ -154,15 +155,17 @@ function copyDir(src, dest, opts, mkDir, srcMode) {
     mkdirSync(dest);
   }
 
-  const dir = opendirSync(src);
+  // Read entries as Buffers when the source is a Buffer path, so non-UTF-8
+  // byte file names survive being joined onto the source and destination.
+  const dir = opendirSync(src, BufferIsBuffer(src) ? { encoding: 'buffer' } : undefined);
 
   try {
     let dirent;
 
     while ((dirent = dir.readSync()) !== null) {
       const { name } = dirent;
-      const srcItem = join(src, name);
-      const destItem = join(dest, name);
+      const srcItem = joinPath(src, name);
+      const destItem = joinPath(dest, name);
       let shouldCopy = true;
 
       if (opts.filter) {

lib/internal/fs/cp/cp.js

@@ -9,8 +9,17 @@ const {
   PromisePrototypeThen,
   PromiseReject,
   SafePromiseAll,
+  StringPrototypeCharCodeAt,
   StringPrototypeSplit,
+  uncurryThis,
 } = primordials;
+const { Buffer } = require('buffer');
+const {
+  concat: BufferConcat,
+  from: BufferFrom,
+  isBuffer: BufferIsBuffer,
+} = Buffer;
+const BufferToString = uncurryThis(Buffer.prototype.toString);
 const {
   codes: {
     ERR_FS_CP_DIR_TO_NON_DIR,
@@ -56,6 +65,34 @@ const {
 } = require('path');
 const fsBinding = internalBinding('fs');
 
+const sepBuffer = BufferFrom(sep);
+const sepCharCode = StringPrototypeCharCodeAt(sep, 0);
+
+// path.resolve()/dirname()/parse() only accept strings. The structural checks
+// (subdirectory and parent-directory detection) work on the decoded path; the
+// copy itself keeps Buffer paths so their bytes survive verbatim.
+function toPathString(path) {
+  return BufferIsBuffer(path) ? BufferToString(path) : path;
+}
+
+// Join a directory path with a directory entry's name. `cp` preserves Buffer
+// paths so non-UTF-8 byte file names on POSIX survive the copy, but path.join()
+// only accepts strings and throws on a Buffer. When either side is a Buffer,
+// concatenate as bytes instead so those names are not mangled or rejected; a
+// separator is inserted unless the base already ends with one.
+function joinPath(base, name) {
+  if (BufferIsBuffer(base) || BufferIsBuffer(name)) {
+    const baseBuffer = BufferIsBuffer(base) ? base : BufferFrom(base);
+    const nameBuffer = BufferIsBuffer(name) ? name : BufferFrom(name);
+    if (baseBuffer.length > 0 &&
+        baseBuffer[baseBuffer.length - 1] === sepCharCode) {
+      return BufferConcat([baseBuffer, nameBuffer]);
+    }
+    return BufferConcat([baseBuffer, sepBuffer, nameBuffer]);
+  }
+  return join(base, name);
+}
+
 async function cpFn(src, dest, opts) {
   // Warn about using preserveTimestamps on 32-bit node
   if (opts.preserveTimestamps && process.arch === 'ia32') {
@@ -138,7 +175,7 @@ function getStats(src, dest, opts) {
 }
 
 async function checkParentDir(destStat, src, dest, opts) {
-  const destParent = dirname(dest);
+  const destParent = dirname(toPathString(dest));
   const dirExists = await pathExists(destParent);
   if (dirExists) return getStatsForCopy(destStat, src, dest, opts);
   await mkdir(destParent, { recursive: true });
@@ -157,8 +194,8 @@ function pathExists(dest) {
 // 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 = resolve(dirname(toPathString(src)));
+  const destParent = resolve(dirname(toPathString(dest)));
   if (destParent === srcParent || destParent === parse(destParent).root) {
     return;
   }
@@ -182,7 +219,7 @@ async function checkParentPaths(src, srcStat, dest) {
 }
 
 const normalizePathToArray = (path) =>
-  ArrayPrototypeFilter(StringPrototypeSplit(resolve(path), sep), Boolean);
+  ArrayPrototypeFilter(StringPrototypeSplit(resolve(toPathString(path)), sep), Boolean);
 
 // Return true if dest is a subdir of src, otherwise false.
 // It only checks the path strings.
@@ -327,11 +364,13 @@ async function mkDirAndCopy(srcMode, src, dest, opts) {
 }
 
 async function copyDir(src, dest, opts) {
-  const dir = await opendir(src);
+  // Read entries as Buffers when the source is a Buffer path, so non-UTF-8
+  // byte file names survive being joined onto the source and destination.
+  const dir = await opendir(src, BufferIsBuffer(src) ? { encoding: 'buffer' } : undefined);
 
   for await (const { name } of dir) {
-    const srcItem = join(src, name);
-    const destItem = join(dest, name);
+    const srcItem = joinPath(src, name);
+    const destItem = joinPath(dest, name);
     const { destStat, skipped } = await checkPaths(srcItem, destItem, opts);
     if (!skipped) await getStatsForCopy(destStat, srcItem, destItem, opts);
   }
@@ -340,7 +379,7 @@ async function copyDir(src, dest, opts) {
 async function onLink(destStat, src, dest, opts) {
   let resolvedSrc = await readlink(src);
   if (!opts.verbatimSymlinks && !isAbsolute(resolvedSrc)) {
-    resolvedSrc = resolve(dirname(src), resolvedSrc);
+    resolvedSrc = resolve(dirname(toPathString(src)), resolvedSrc);
   }
   const srcIsDir = fsBinding.internalModuleStat(src) === 1;
   const symlinkType = srcIsDir ? 'dir' : 'file';
@@ -360,7 +399,7 @@ async function onLink(destStat, src, dest, opts) {
     throw err;
   }
   if (!isAbsolute(resolvedDest)) {
-    resolvedDest = resolve(dirname(dest), resolvedDest);
+    resolvedDest = resolve(dirname(toPathString(dest)), resolvedDest);
   }
 
   if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
@@ -398,4 +437,5 @@ module.exports = {
   areIdentical,
   cpFn,
   isSrcSubdir,
+  joinPath,
 };

test/parallel/test-fs-cp-async-buffer.js

@@ -0,0 +1,26 @@
+'use strict';
+
+// Refs: https://github.com/nodejs/node/issues/58634
+// fs.promises.cp() must accept Buffer paths for src and dest, matching
+// fs.cpSync(), and copy the directory tree.
+
+const common = require('../common');
+const assert = require('assert');
+const { mkdirSync, writeFileSync, readFileSync, promises } = require('fs');
+const { join } = require('path');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+
+const src = join(tmpdir.path, 'a');
+const dest = join(tmpdir.path, 'b');
+mkdirSync(join(src, 'sub'), { recursive: true });
+writeFileSync(join(src, 'file.txt'), 'hello');
+writeFileSync(join(src, 'sub', 'nested.txt'), 'world');
+
+promises.cp(Buffer.from(src), Buffer.from(dest), { recursive: true })
+  .then(common.mustCall(() => {
+    assert.strictEqual(readFileSync(join(dest, 'file.txt'), 'utf8'), 'hello');
+    assert.strictEqual(
+      readFileSync(join(dest, 'sub', 'nested.txt'), 'utf8'), 'world');
+  }));

test/parallel/test-fs-cp-buffer-non-utf8.js

@@ -0,0 +1,39 @@
+'use strict';
+
+// Refs: https://github.com/nodejs/node/issues/58634
+// With Buffer paths, fs.cpSync() copies files whose names are not valid UTF-8
+// (which are permitted on POSIX) without mangling them.
+
+const common = require('../common');
+
+if (!common.isLinux) {
+  common.skip('non-UTF-8 file names are only valid on Linux');
+}
+
+const assert = require('assert');
+const { join, sep } = require('path');
+const {
+  cpSync, mkdirSync, writeFileSync, readFileSync, existsSync,
+} = require('fs');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+
+const src = Buffer.from(join(tmpdir.path, 'a'));
+const dest = Buffer.from(join(tmpdir.path, 'b'));
+mkdirSync(src, { recursive: true });
+
+// Shift-JIS encoding of こんにちは世界 ("Hello, World"); not valid UTF-8.
+const name = Buffer.from([
+  0x82, 0xB1, 0x82, 0xF1, 0x82, 0xC9, 0x82,
+  0xBF, 0x82, 0xCD, 0x90, 0x6C, 0x8C, 0x8E,
+]);
+const sepBuf = Buffer.from(sep);
+const srcFile = Buffer.concat([src, sepBuf, name]);
+writeFileSync(srcFile, 'content');
+
+cpSync(src, dest, { recursive: true });
+
+const destFile = Buffer.concat([dest, sepBuf, name]);
+assert.ok(existsSync(destFile));
+assert.strictEqual(readFileSync(destFile, 'utf8'), 'content');

test/parallel/test-fs-cp-filter.js

@@ -0,0 +1,30 @@
+'use strict';
+
+// Refs: https://github.com/nodejs/node/issues/58634
+// fs.cpSync() must accept Buffer paths together with a filter function when
+// recursively copying directories. The filter receives Buffer paths, and the
+// tree is copied.
+
+const common = require('../common');
+const assert = require('assert');
+const { cpSync, mkdirSync, writeFileSync, readFileSync } = require('fs');
+const { join } = require('path');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+
+const src = join(tmpdir.path, 'a');
+const dest = join(tmpdir.path, 'b');
+mkdirSync(join(src, 'c'), { recursive: true });
+writeFileSync(join(src, 'c', 'file.txt'), 'data');
+
+cpSync(Buffer.from(src), Buffer.from(dest), {
+  recursive: true,
+  filter: common.mustCallAtLeast((srcArg, destArg) => {
+    assert.ok(Buffer.isBuffer(srcArg));
+    assert.ok(Buffer.isBuffer(destArg));
+    return true;
+  }, 1),
+});
+
+assert.strictEqual(readFileSync(join(dest, 'c', 'file.txt'), 'utf8'), 'data');