Skip to content

Commit 5b8530b

Browse files
committed
fs: accept Buffer paths in fs.promises.cp
fs.promises.cp threw ERR_INVALID_ARG_TYPE when given Buffer paths, while fs.cpSync accepted them. The async helper at lib/internal/fs/cp/cp.js calls path.resolve/dirname/join on the raw input, and the path module rejects non-strings. The sync path lives in C++ and accepts Buffer transparently via BufferValue. Convert Buffer/Uint8Array paths to UTF-8 strings at the entry of cpFn so the async implementation matches the documented string | Buffer | URL contract and the sync behavior. URL paths are already pre-converted by getValidatedPath upstream and so are unaffected. Fixes: #58869 Signed-off-by: Maruthan G <[email protected]>
1 parent 21436f0 commit 5b8530b

2 files changed

Lines changed: 121 additions & 0 deletions

File tree

lib/internal/fs/cp/cp.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const {
1111
SafePromiseAll,
1212
StringPrototypeSplit,
1313
} = primordials;
14+
const { Buffer } = require('buffer');
15+
const { isUint8Array } = require('internal/util/types');
1416
const {
1517
codes: {
1618
ERR_FS_CP_DIR_TO_NON_DIR,
@@ -56,13 +58,27 @@ const {
5658
} = require('path');
5759
const fsBinding = internalBinding('fs');
5860

61+
// The path module functions used in this file (resolve, dirname, join, etc.)
62+
// only accept strings. Convert Buffer paths to strings here so that the async
63+
// cp behaves consistently with the synchronous version, which accepts both
64+
// string and Buffer paths via its C++ binding.
65+
// Refs: https://github.com/nodejs/node/issues/58869
66+
function toPathString(path) {
67+
if (isUint8Array(path)) {
68+
return Buffer.from(path).toString();
69+
}
70+
return path;
71+
}
72+
5973
async function cpFn(src, dest, opts) {
6074
// Warn about using preserveTimestamps on 32-bit node
6175
if (opts.preserveTimestamps && process.arch === 'ia32') {
6276
const warning = 'Using the preserveTimestamps option in 32-bit ' +
6377
'node is not recommended';
6478
process.emitWarning(warning, 'TimestampPrecisionWarning');
6579
}
80+
src = toPathString(src);
81+
dest = toPathString(dest);
6682
const stats = await checkPaths(src, dest, opts);
6783
const { srcStat, destStat, skipped } = stats;
6884
if (skipped) return;
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
'use strict';
2+
3+
// This tests that fs.cp, fs.promises.cp, and fs.cpSync all accept Buffer
4+
// paths for `src` and `dest` (matching the documented `string | Buffer | URL`
5+
// type). Prior to the fix for https://github.com/nodejs/node/issues/58869,
6+
// the async variants threw `ERR_INVALID_ARG_TYPE` because the internal
7+
// implementation called `path.resolve` on a Buffer, while the sync variant
8+
// already accepted Buffer paths via the C++ binding.
9+
10+
const common = require('../common');
11+
const assert = require('node:assert');
12+
const fs = require('node:fs');
13+
const path = require('node:path');
14+
const { Buffer } = require('node:buffer');
15+
const { pathToFileURL } = require('node:url');
16+
const tmpdir = require('../common/tmpdir');
17+
18+
tmpdir.refresh();
19+
20+
let counter = 0;
21+
function makePaths(prefix) {
22+
const id = `${prefix}-${process.pid}-${++counter}`;
23+
return {
24+
src: tmpdir.resolve(`${id}-src.txt`),
25+
dest: tmpdir.resolve(`${id}-dest.txt`),
26+
};
27+
}
28+
29+
function writeSrc(srcPath) {
30+
fs.writeFileSync(srcPath, 'cp-buffer-path');
31+
}
32+
33+
function assertCopied(srcPath, destPath) {
34+
assert.strictEqual(
35+
fs.readFileSync(destPath, 'utf8'),
36+
fs.readFileSync(srcPath, 'utf8'),
37+
);
38+
}
39+
40+
(async () => {
41+
// 1. fs.promises.cp() accepts Buffer src and Buffer dest. This used to
42+
// throw ERR_INVALID_ARG_TYPE.
43+
{
44+
const { src, dest } = makePaths('promises-buf');
45+
writeSrc(src);
46+
await fs.promises.cp(Buffer.from(src), Buffer.from(dest));
47+
assertCopied(src, dest);
48+
}
49+
50+
// 2. Mixed forms: Buffer src + string dest, and string src + Buffer dest.
51+
{
52+
const { src, dest } = makePaths('promises-mixed1');
53+
writeSrc(src);
54+
await fs.promises.cp(Buffer.from(src), dest);
55+
assertCopied(src, dest);
56+
}
57+
58+
{
59+
const { src, dest } = makePaths('promises-mixed2');
60+
writeSrc(src);
61+
await fs.promises.cp(src, Buffer.from(dest));
62+
assertCopied(src, dest);
63+
}
64+
65+
// 3. fs.cpSync() continues to accept Buffer src and Buffer dest (the
66+
// existing documented behavior); guard against regressions there too.
67+
{
68+
const { src, dest } = makePaths('sync-buf');
69+
writeSrc(src);
70+
fs.cpSync(Buffer.from(src), Buffer.from(dest));
71+
assertCopied(src, dest);
72+
}
73+
74+
// 4. URL paths still work for the async variants.
75+
{
76+
const { src, dest } = makePaths('promises-url');
77+
writeSrc(src);
78+
await fs.promises.cp(pathToFileURL(src), pathToFileURL(dest));
79+
assertCopied(src, dest);
80+
}
81+
82+
// 5. Recursive directory copy with Buffer paths via fs.promises.cp.
83+
{
84+
const id = `dir-${process.pid}-${++counter}`;
85+
const srcDir = tmpdir.resolve(`${id}-src`);
86+
const destDir = tmpdir.resolve(`${id}-dest`);
87+
fs.mkdirSync(srcDir, { recursive: true });
88+
fs.writeFileSync(path.join(srcDir, 'a.txt'), 'a');
89+
fs.writeFileSync(path.join(srcDir, 'b.txt'), 'b');
90+
await fs.promises.cp(Buffer.from(srcDir), Buffer.from(destDir),
91+
{ recursive: true });
92+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'a.txt'), 'utf8'), 'a');
93+
assert.strictEqual(fs.readFileSync(path.join(destDir, 'b.txt'), 'utf8'), 'b');
94+
}
95+
96+
// 6. fs.cp() (callback) accepts Buffer src and Buffer dest. Since fs.cp is
97+
// a callbackified version of fs.promises.cp, this exercises the same fix.
98+
{
99+
const { src, dest } = makePaths('cb-buf');
100+
writeSrc(src);
101+
fs.cp(Buffer.from(src), Buffer.from(dest), common.mustSucceed(() => {
102+
assertCopied(src, dest);
103+
}));
104+
}
105+
})().then(common.mustCall());

0 commit comments

Comments
 (0)