Skip to content

Commit

Permalink
fs: check subdir correctly in cpSync
Browse files Browse the repository at this point in the history
PR-URL: #55033
Fixes: #54285
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
jazelly authored and targos committed Oct 4, 2024
1 parent 9764002 commit a43ce87
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 4 deletions.
7 changes: 5 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3111,9 +3111,12 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
}

std::u8string dest_path_str = dest_path.u8string();

std::u8string src_path_str = src_path.u8string();
if (!src_path_str.ends_with(std::filesystem::path::preferred_separator)) {
src_path_str += std::filesystem::path::preferred_separator;
}
// Check if dest_path is a subdirectory of src_path.
if (src_is_dir && dest_path_str.starts_with(src_path.u8string())) {
if (src_is_dir && dest_path_str.starts_with(src_path_str)) {
std::u8string message = u8"Cannot copy " + src_path.u8string() +
u8" to a subdirectory of self " +
dest_path.u8string();
Expand Down
43 changes: 41 additions & 2 deletions test/parallel/test-fs-cp.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import tmpdir from '../common/tmpdir.js';
tmpdir.refresh();

let dirc = 0;
function nextdir() {
return tmpdir.resolve(`copy_${++dirc}`);
function nextdir(dirname) {
return tmpdir.resolve(dirname || `copy_${++dirc}`);
}

// Synchronous implementation of copy.
Expand Down Expand Up @@ -320,6 +320,45 @@ function nextdir() {
);
}

// It must not throw error if attempt is made to copy to dest
// directory with same prefix as src directory
// regression test for https://github.com/nodejs/node/issues/54285
{
const src = nextdir('prefix');
const dest = nextdir('prefix-a');
mkdirSync(src);
mkdirSync(dest);
cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
}

// It must not throw error if attempt is made to copy to dest
// directory if the parent of dest has same prefix as src directory
// regression test for https://github.com/nodejs/node/issues/54285
{
const src = nextdir('aa');
const destParent = nextdir('aaa');
const dest = nextdir('aaa/aabb');
mkdirSync(src);
mkdirSync(destParent);
mkdirSync(dest);
cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
}

// It throws error if attempt is made to copy src to dest
// when src is parent directory of the parent of dest
{
const src = nextdir('a');
const destParent = nextdir('a/b');
const dest = nextdir('a/b/c');
mkdirSync(src);
mkdirSync(destParent);
mkdirSync(dest);
assert.throws(
() => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
{ code: 'ERR_FS_CP_EINVAL' },
);
}

// It throws error if attempt is made to copy to subdirectory of self.
{
const src = './test/fixtures/copy/kitchen-sink';
Expand Down

0 comments on commit a43ce87

Please sign in to comment.