Skip to content

Commit 004d3c3

Browse files
fs: support Buffer paths in cp() and cpSync()
`fs.cpSync()` with a filter, and `fs.promises.cp()`, threw `ERR_INVALID_ARG_TYPE` when `src` or `dest` was a Buffer. The recursive directory walk passed the Buffer paths to `path.join()`, and the async path additionally to `path.resolve()` and `path.dirname()`, all of which only accept strings. The sync path only reached this on the filter branch because the no-filter branch runs entirely in C++. Join directory entries onto Buffer paths by concatenating bytes, read entries with `encoding: 'buffer'` so non-UTF-8 byte file names on POSIX are preserved, and decode Buffer paths to strings only for the structural subdirectory and parent-directory checks. Fixes: #58634 Assisted-by: Claude Code Signed-off-by: Huzaifa Abdul Rehman <huzaifarehman897@gmail.com>
1 parent e68a93a commit 004d3c3

7 files changed

Lines changed: 152 additions & 71 deletions

File tree

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
// This file is a modified version of the fs-extra's copySync method.
44

55
const fsBinding = internalBinding('fs');
6-
const { isSrcSubdir } = require('internal/fs/cp/cp');
6+
const { isSrcSubdir, joinPath } = require('internal/fs/cp/cp');
7+
const { Buffer } = require('buffer');
8+
const { isBuffer: BufferIsBuffer } = Buffer;
79
const { codes: {
810
ERR_FS_CP_EEXIST,
911
ERR_FS_CP_EINVAL,
@@ -33,7 +35,6 @@ const {
3335
const {
3436
dirname,
3537
isAbsolute,
36-
join,
3738
resolve,
3839
} = require('path');
3940
const { isPromise } = require('util/types');
@@ -154,15 +155,17 @@ function copyDir(src, dest, opts, mkDir, srcMode) {
154155
mkdirSync(dest);
155156
}
156157

157-
const dir = opendirSync(src);
158+
// Read entries as Buffers when the source is a Buffer path, so non-UTF-8
159+
// byte file names survive being joined onto the source and destination.
160+
const dir = opendirSync(src, BufferIsBuffer(src) ? { encoding: 'buffer' } : undefined);
158161

159162
try {
160163
let dirent;
161164

162165
while ((dirent = dir.readSync()) !== null) {
163166
const { name } = dirent;
164-
const srcItem = join(src, name);
165-
const destItem = join(dest, name);
167+
const srcItem = joinPath(src, name);
168+
const destItem = joinPath(dest, name);
166169
let shouldCopy = true;
167170

168171
if (opts.filter) {

lib/internal/fs/cp/cp.js

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,17 @@ const {
99
PromisePrototypeThen,
1010
PromiseReject,
1111
SafePromiseAll,
12+
StringPrototypeCharCodeAt,
1213
StringPrototypeSplit,
14+
uncurryThis,
1315
} = primordials;
16+
const { Buffer } = require('buffer');
17+
const {
18+
concat: BufferConcat,
19+
from: BufferFrom,
20+
isBuffer: BufferIsBuffer,
21+
} = Buffer;
22+
const BufferToString = uncurryThis(Buffer.prototype.toString);
1423
const {
1524
codes: {
1625
ERR_FS_CP_DIR_TO_NON_DIR,
@@ -56,6 +65,34 @@ const {
5665
} = require('path');
5766
const fsBinding = internalBinding('fs');
5867

68+
const sepBuffer = BufferFrom(sep);
69+
const sepCharCode = StringPrototypeCharCodeAt(sep, 0);
70+
71+
// path.resolve()/dirname()/parse() only accept strings. The structural checks
72+
// (subdirectory and parent-directory detection) work on the decoded path; the
73+
// copy itself keeps Buffer paths so their bytes survive verbatim.
74+
function toPathString(path) {
75+
return BufferIsBuffer(path) ? BufferToString(path) : path;
76+
}
77+
78+
// Join a directory path with a directory entry's name. `cp` preserves Buffer
79+
// paths so non-UTF-8 byte file names on POSIX survive the copy, but path.join()
80+
// only accepts strings and throws on a Buffer. When either side is a Buffer,
81+
// concatenate as bytes instead so those names are not mangled or rejected; a
82+
// separator is inserted unless the base already ends with one.
83+
function joinPath(base, name) {
84+
if (BufferIsBuffer(base) || BufferIsBuffer(name)) {
85+
const baseBuffer = BufferIsBuffer(base) ? base : BufferFrom(base);
86+
const nameBuffer = BufferIsBuffer(name) ? name : BufferFrom(name);
87+
if (baseBuffer.length > 0 &&
88+
baseBuffer[baseBuffer.length - 1] === sepCharCode) {
89+
return BufferConcat([baseBuffer, nameBuffer]);
90+
}
91+
return BufferConcat([baseBuffer, sepBuffer, nameBuffer]);
92+
}
93+
return join(base, name);
94+
}
95+
5996
async function cpFn(src, dest, opts) {
6097
// Warn about using preserveTimestamps on 32-bit node
6198
if (opts.preserveTimestamps && process.arch === 'ia32') {
@@ -138,7 +175,7 @@ function getStats(src, dest, opts) {
138175
}
139176

140177
async function checkParentDir(destStat, src, dest, opts) {
141-
const destParent = dirname(dest);
178+
const destParent = dirname(toPathString(dest));
142179
const dirExists = await pathExists(destParent);
143180
if (dirExists) return getStatsForCopy(destStat, src, dest, opts);
144181
await mkdir(destParent, { recursive: true });
@@ -157,8 +194,8 @@ function pathExists(dest) {
157194
// checks the src and dest inodes. It starts from the deepest
158195
// parent and stops once it reaches the src parent or the root path.
159196
async function checkParentPaths(src, srcStat, dest) {
160-
const srcParent = resolve(dirname(src));
161-
const destParent = resolve(dirname(dest));
197+
const srcParent = resolve(dirname(toPathString(src)));
198+
const destParent = resolve(dirname(toPathString(dest)));
162199
if (destParent === srcParent || destParent === parse(destParent).root) {
163200
return;
164201
}
@@ -182,7 +219,7 @@ async function checkParentPaths(src, srcStat, dest) {
182219
}
183220

184221
const normalizePathToArray = (path) =>
185-
ArrayPrototypeFilter(StringPrototypeSplit(resolve(path), sep), Boolean);
222+
ArrayPrototypeFilter(StringPrototypeSplit(resolve(toPathString(path)), sep), Boolean);
186223

187224
// Return true if dest is a subdir of src, otherwise false.
188225
// It only checks the path strings.
@@ -327,11 +364,13 @@ async function mkDirAndCopy(srcMode, src, dest, opts) {
327364
}
328365

329366
async function copyDir(src, dest, opts) {
330-
const dir = await opendir(src);
367+
// Read entries as Buffers when the source is a Buffer path, so non-UTF-8
368+
// byte file names survive being joined onto the source and destination.
369+
const dir = await opendir(src, BufferIsBuffer(src) ? { encoding: 'buffer' } : undefined);
331370

332371
for await (const { name } of dir) {
333-
const srcItem = join(src, name);
334-
const destItem = join(dest, name);
372+
const srcItem = joinPath(src, name);
373+
const destItem = joinPath(dest, name);
335374
const { destStat, skipped } = await checkPaths(srcItem, destItem, opts);
336375
if (!skipped) await getStatsForCopy(destStat, srcItem, destItem, opts);
337376
}
@@ -340,7 +379,7 @@ async function copyDir(src, dest, opts) {
340379
async function onLink(destStat, src, dest, opts) {
341380
let resolvedSrc = await readlink(src);
342381
if (!opts.verbatimSymlinks && !isAbsolute(resolvedSrc)) {
343-
resolvedSrc = resolve(dirname(src), resolvedSrc);
382+
resolvedSrc = resolve(dirname(toPathString(src)), resolvedSrc);
344383
}
345384
const srcIsDir = fsBinding.internalModuleStat(src) === 1;
346385
const symlinkType = srcIsDir ? 'dir' : 'file';
@@ -360,7 +399,7 @@ async function onLink(destStat, src, dest, opts) {
360399
throw err;
361400
}
362401
if (!isAbsolute(resolvedDest)) {
363-
resolvedDest = resolve(dirname(dest), resolvedDest);
402+
resolvedDest = resolve(dirname(toPathString(dest)), resolvedDest);
364403
}
365404

366405
if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
@@ -398,4 +437,5 @@ module.exports = {
398437
areIdentical,
399438
cpFn,
400439
isSrcSubdir,
440+
joinPath,
401441
};

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

Lines changed: 0 additions & 23 deletions
This file was deleted.

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

Lines changed: 0 additions & 34 deletions
This file was deleted.
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict';
2+
3+
// Refs: https://github.com/nodejs/node/issues/58634
4+
// fs.promises.cp() must accept Buffer paths for src and dest, matching
5+
// fs.cpSync(), and copy the directory tree.
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
const { mkdirSync, writeFileSync, readFileSync, promises } = require('fs');
10+
const { join } = require('path');
11+
const tmpdir = require('../common/tmpdir');
12+
13+
tmpdir.refresh();
14+
15+
const src = join(tmpdir.path, 'a');
16+
const dest = join(tmpdir.path, 'b');
17+
mkdirSync(join(src, 'sub'), { recursive: true });
18+
writeFileSync(join(src, 'file.txt'), 'hello');
19+
writeFileSync(join(src, 'sub', 'nested.txt'), 'world');
20+
21+
promises.cp(Buffer.from(src), Buffer.from(dest), { recursive: true })
22+
.then(common.mustCall(() => {
23+
assert.strictEqual(readFileSync(join(dest, 'file.txt'), 'utf8'), 'hello');
24+
assert.strictEqual(
25+
readFileSync(join(dest, 'sub', 'nested.txt'), 'utf8'), 'world');
26+
}));
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
'use strict';
2+
3+
// Refs: https://github.com/nodejs/node/issues/58634
4+
// With Buffer paths, fs.cpSync() copies files whose names are not valid UTF-8
5+
// (which are permitted on POSIX) without mangling them.
6+
7+
const common = require('../common');
8+
9+
if (!common.isLinux) {
10+
common.skip('non-UTF-8 file names are only valid on Linux');
11+
}
12+
13+
const assert = require('assert');
14+
const { join, sep } = require('path');
15+
const {
16+
cpSync, mkdirSync, writeFileSync, readFileSync, existsSync,
17+
} = require('fs');
18+
const tmpdir = require('../common/tmpdir');
19+
20+
tmpdir.refresh();
21+
22+
const src = Buffer.from(join(tmpdir.path, 'a'));
23+
const dest = Buffer.from(join(tmpdir.path, 'b'));
24+
mkdirSync(src, { recursive: true });
25+
26+
// Shift-JIS encoding of こんにちは世界 ("Hello, World"); not valid UTF-8.
27+
const name = Buffer.from([
28+
0x82, 0xB1, 0x82, 0xF1, 0x82, 0xC9, 0x82,
29+
0xBF, 0x82, 0xCD, 0x90, 0x6C, 0x8C, 0x8E,
30+
]);
31+
const sepBuf = Buffer.from(sep);
32+
const srcFile = Buffer.concat([src, sepBuf, name]);
33+
writeFileSync(srcFile, 'content');
34+
35+
cpSync(src, dest, { recursive: true });
36+
37+
const destFile = Buffer.concat([dest, sepBuf, name]);
38+
assert.ok(existsSync(destFile));
39+
assert.strictEqual(readFileSync(destFile, 'utf8'), 'content');

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
'use strict';
2+
3+
// Refs: https://github.com/nodejs/node/issues/58634
4+
// fs.cpSync() must accept Buffer paths together with a filter function when
5+
// recursively copying directories. The filter receives Buffer paths, and the
6+
// tree is copied.
7+
8+
const common = require('../common');
9+
const assert = require('assert');
10+
const { cpSync, mkdirSync, writeFileSync, readFileSync } = require('fs');
11+
const { join } = require('path');
12+
const tmpdir = require('../common/tmpdir');
13+
14+
tmpdir.refresh();
15+
16+
const src = join(tmpdir.path, 'a');
17+
const dest = join(tmpdir.path, 'b');
18+
mkdirSync(join(src, 'c'), { recursive: true });
19+
writeFileSync(join(src, 'c', 'file.txt'), 'data');
20+
21+
cpSync(Buffer.from(src), Buffer.from(dest), {
22+
recursive: true,
23+
filter: common.mustCallAtLeast((srcArg, destArg) => {
24+
assert.ok(Buffer.isBuffer(srcArg));
25+
assert.ok(Buffer.isBuffer(destArg));
26+
return true;
27+
}, 1),
28+
});
29+
30+
assert.strictEqual(readFileSync(join(dest, 'c', 'file.txt'), 'utf8'), 'data');

0 commit comments

Comments
 (0)