From 9358d9e94de51acb88b173f55e490e0eecbd179a Mon Sep 17 00:00:00 2001 From: Jason Zhang Date: Sat, 21 Sep 2024 06:31:41 +0930 Subject: [PATCH] fs: check subdir correctly in cpSync --- src/node_file.cc | 7 ++++-- test/parallel/test-fs-cp.mjs | 43 ++++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 57ddb2c13bd68f..bd30c0cb6c56cd 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3204,9 +3204,12 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& 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(); diff --git a/test/parallel/test-fs-cp.mjs b/test/parallel/test-fs-cp.mjs index 54bb3f95bf40d5..d34160aa5ace37 100644 --- a/test/parallel/test-fs-cp.mjs +++ b/test/parallel/test-fs-cp.mjs @@ -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. @@ -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';