diff --git a/lib/internal/fs/cp/cp.js b/lib/internal/fs/cp/cp.js index 10c52b114634..3f0e05967d18 100644 --- a/lib/internal/fs/cp/cp.js +++ b/lib/internal/fs/cp/cp.js @@ -189,7 +189,8 @@ const normalizePathToArray = (path) => function isSrcSubdir(src, dest) { const srcArr = normalizePathToArray(src); const destArr = normalizePathToArray(dest); - return ArrayPrototypeEvery(srcArr, (cur, i) => destArr[i] === cur); + return srcArr.length < destArr.length && + ArrayPrototypeEvery(srcArr, (cur, i) => destArr[i] === cur); } async function getStatsForCopy(destStat, src, dest, opts) { diff --git a/src/node_file.cc b/src/node_file.cc index ae0d9f34f8e1..1717ed86daad 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3702,7 +3702,7 @@ bool isInsideDir(const std::filesystem::path& src, const std::filesystem::path& dest) { auto srcArr = normalizePathToArray(src); auto destArr = normalizePathToArray(dest); - if (srcArr.size() > destArr.size()) return false; + if (srcArr.size() >= destArr.size()) return false; return std::equal(srcArr.begin(), srcArr.end(), destArr.begin()); } diff --git a/test/parallel/test-fs-cp-async-symlink-points-to-dest.mjs b/test/parallel/test-fs-cp-async-symlink-points-to-dest.mjs index f8e60fe1fa2b..9ca0cdb8e190 100644 --- a/test/parallel/test-fs-cp-async-symlink-points-to-dest.mjs +++ b/test/parallel/test-fs-cp-async-symlink-points-to-dest.mjs @@ -1,4 +1,5 @@ -// This tests that cp() returns error if symlink in src points to location in dest. +// This tests that cp() allows matching symlink targets but returns an error +// when the destination target is a subdirectory of the source target. import { mustCall, mustNotMutateObjectDeep } from '../common/index.mjs'; import { nextdir } from '../common/fs.js'; @@ -9,12 +10,27 @@ import tmpdir from '../common/tmpdir.js'; tmpdir.refresh(); -const src = nextdir(); -mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); -const dest = nextdir(); -mkdirSync(dest); -symlinkSync(dest, join(src, 'link')); -cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })); -cp(src, dest, mustNotMutateObjectDeep({ recursive: true }), mustCall((err) => { - assert.strictEqual(err.code, 'ERR_FS_CP_EINVAL'); -})); +{ + const src = nextdir(); + mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); + const dest = nextdir(); + mkdirSync(dest); + symlinkSync(dest, join(src, 'link')); + cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })); + cp(src, dest, mustNotMutateObjectDeep({ recursive: true }), mustCall((err) => { + assert.ifError(err); + })); +} + +{ + const src = nextdir(); + mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); + const dest = nextdir(); + const destSubdir = join(dest, 'subdir'); + mkdirSync(destSubdir, mustNotMutateObjectDeep({ recursive: true })); + symlinkSync(dest, join(src, 'link')); + symlinkSync(destSubdir, join(dest, 'link')); + cp(src, dest, mustNotMutateObjectDeep({ recursive: true }), mustCall((err) => { + assert.strictEqual(err.code, 'ERR_FS_CP_EINVAL'); + })); +} diff --git a/test/parallel/test-fs-cp-same-symlink-target.mjs b/test/parallel/test-fs-cp-same-symlink-target.mjs new file mode 100644 index 000000000000..ccfc1af23069 --- /dev/null +++ b/test/parallel/test-fs-cp-same-symlink-target.mjs @@ -0,0 +1,51 @@ +// This tests that copying the same directory twice succeeds when it contains a +// symlink to a directory. +import { mustNotMutateObjectDeep } from '../common/index.mjs'; +import { nextdir } from '../common/fs.js'; +import assert from 'node:assert'; +import { + cp, + cpSync, + lstatSync, + mkdirSync, + realpathSync, + symlinkSync, +} from 'node:fs'; +import { cp as cpPromise } from 'node:fs/promises'; +import { join } from 'node:path'; +import { promisify } from 'node:util'; + +import tmpdir from '../common/tmpdir.js'; +tmpdir.refresh(); + +const target = nextdir(); +const src = nextdir(); +mkdirSync(target, mustNotMutateObjectDeep({ recursive: true })); +mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); +symlinkSync(target, join(src, 'link')); + +const options = () => mustNotMutateObjectDeep({ recursive: true }); +const filterOptions = () => mustNotMutateObjectDeep({ + filter: () => true, + recursive: true, +}); +const copyCallback = promisify(cp); +const destinations = [nextdir(), nextdir(), nextdir(), nextdir()]; + +cpSync(src, destinations[0], options()); +cpSync(src, destinations[0], options()); + +cpSync(src, destinations[1], filterOptions()); +cpSync(src, destinations[1], filterOptions()); + +await copyCallback(src, destinations[2], options()); +await copyCallback(src, destinations[2], options()); + +await cpPromise(src, destinations[3], options()); +await cpPromise(src, destinations[3], options()); + +for (const dest of destinations) { + const link = join(dest, 'link'); + assert(lstatSync(link).isSymbolicLink()); + assert.strictEqual(realpathSync(link), realpathSync(target)); +} diff --git a/test/parallel/test-fs-cp-sync-symlink-points-to-dest-error.mjs b/test/parallel/test-fs-cp-sync-symlink-points-to-dest-error.mjs index 141798f1d27d..7d6d2e0c7c5c 100644 --- a/test/parallel/test-fs-cp-sync-symlink-points-to-dest-error.mjs +++ b/test/parallel/test-fs-cp-sync-symlink-points-to-dest-error.mjs @@ -1,4 +1,5 @@ -// This tests that cpSync throws error if symlink in src points to location in dest. +// This tests that cpSync allows matching symlink targets but throws when the +// destination target is a subdirectory of the source target. import { mustNotMutateObjectDeep } from '../common/index.mjs'; import { nextdir } from '../common/fs.js'; import assert from 'node:assert'; @@ -8,15 +9,28 @@ import { join } from 'node:path'; import tmpdir from '../common/tmpdir.js'; tmpdir.refresh(); -const src = nextdir(); -mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); -const dest = nextdir(); -mkdirSync(dest); -symlinkSync(dest, join(src, 'link')); -cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })); -assert.throws( - () => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })), - { - code: 'ERR_FS_CP_EINVAL' - } -); +{ + const src = nextdir(); + mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); + const dest = nextdir(); + mkdirSync(dest); + symlinkSync(dest, join(src, 'link')); + cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })); + cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })); +} + +{ + const src = nextdir(); + mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); + const dest = nextdir(); + const destSubdir = join(dest, 'subdir'); + mkdirSync(destSubdir, mustNotMutateObjectDeep({ recursive: true })); + symlinkSync(dest, join(src, 'link')); + symlinkSync(destSubdir, join(dest, 'link')); + assert.throws( + () => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })), + { + code: 'ERR_FS_CP_EINVAL' + } + ); +}