fs: fix repeated copies of directory symlinks

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) {

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());
 }
 

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');
+  }));
+}

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));
+}

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'
+    }
+  );
+}