Skip to content

Commit 7fe5bcd

Browse files
authored
fs: check subdir correctly in cpSync
PR-URL: #55033 Fixes: #54285 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 2d77ba5 commit 7fe5bcd

File tree

2 files changed

+46
-4
lines changed

2 files changed

+46
-4
lines changed

src/node_file.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -3206,9 +3206,12 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32063206
}
32073207

32083208
std::u8string dest_path_str = dest_path.u8string();
3209-
3209+
std::u8string src_path_str = src_path.u8string();
3210+
if (!src_path_str.ends_with(std::filesystem::path::preferred_separator)) {
3211+
src_path_str += std::filesystem::path::preferred_separator;
3212+
}
32103213
// Check if dest_path is a subdirectory of src_path.
3211-
if (src_is_dir && dest_path_str.starts_with(src_path.u8string())) {
3214+
if (src_is_dir && dest_path_str.starts_with(src_path_str)) {
32123215
std::u8string message = u8"Cannot copy " + src_path.u8string() +
32133216
u8" to a subdirectory of self " +
32143217
dest_path.u8string();

test/parallel/test-fs-cp.mjs

+41-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ import tmpdir from '../common/tmpdir.js';
2424
tmpdir.refresh();
2525

2626
let dirc = 0;
27-
function nextdir() {
28-
return tmpdir.resolve(`copy_${++dirc}`);
27+
function nextdir(dirname) {
28+
return tmpdir.resolve(dirname || `copy_${++dirc}`);
2929
}
3030

3131
// Synchronous implementation of copy.
@@ -320,6 +320,45 @@ function nextdir() {
320320
);
321321
}
322322

323+
// It must not throw error if attempt is made to copy to dest
324+
// directory with same prefix as src directory
325+
// regression test for https://github.com/nodejs/node/issues/54285
326+
{
327+
const src = nextdir('prefix');
328+
const dest = nextdir('prefix-a');
329+
mkdirSync(src);
330+
mkdirSync(dest);
331+
cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
332+
}
333+
334+
// It must not throw error if attempt is made to copy to dest
335+
// directory if the parent of dest has same prefix as src directory
336+
// regression test for https://github.com/nodejs/node/issues/54285
337+
{
338+
const src = nextdir('aa');
339+
const destParent = nextdir('aaa');
340+
const dest = nextdir('aaa/aabb');
341+
mkdirSync(src);
342+
mkdirSync(destParent);
343+
mkdirSync(dest);
344+
cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
345+
}
346+
347+
// It throws error if attempt is made to copy src to dest
348+
// when src is parent directory of the parent of dest
349+
{
350+
const src = nextdir('a');
351+
const destParent = nextdir('a/b');
352+
const dest = nextdir('a/b/c');
353+
mkdirSync(src);
354+
mkdirSync(destParent);
355+
mkdirSync(dest);
356+
assert.throws(
357+
() => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
358+
{ code: 'ERR_FS_CP_EINVAL' },
359+
);
360+
}
361+
323362
// It throws error if attempt is made to copy to subdirectory of self.
324363
{
325364
const src = './test/fixtures/copy/kitchen-sink';

0 commit comments

Comments
 (0)