From 980e96afe468adeeb6312a0eaa1501289add3821 Mon Sep 17 00:00:00 2001 From: Tho Date: Sun, 25 Sep 2022 18:36:24 +0800 Subject: [PATCH 1/3] fs: fix operation not permitted fix: #44720 issue: - `copyDir()` calls `checkPathsSync()`, which invokes `lstat()` which causes error because of not checking the opts.filter changes: - check opts.filter before calling `checkPathsSync` and copy logic - cleanup `startCopy` function --- lib/internal/fs/cp/cp-sync.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/internal/fs/cp/cp-sync.js b/lib/internal/fs/cp/cp-sync.js index f9d159a193107e..20c77fc7cb1631 100644 --- a/lib/internal/fs/cp/cp-sync.js +++ b/lib/internal/fs/cp/cp-sync.js @@ -158,11 +158,6 @@ function handleFilterAndCopy(destStat, src, dest, opts) { return getStats(destStat, src, dest, opts); } -function startCopy(destStat, src, dest, opts) { - if (opts.filter && !opts.filter(src, dest)) return; - return getStats(destStat, src, dest, opts); -} - function getStats(destStat, src, dest, opts) { const statSyncFn = opts.dereference ? statSync : lstatSync; const srcStat = statSyncFn(src); @@ -284,9 +279,9 @@ function copyDir(src, dest, opts) { const { name } = dirent; const srcItem = join(src, name); const destItem = join(dest, name); + if (opts.filter && !opts.filter(srcItem, destItem)) continue; const { destStat } = checkPathsSync(srcItem, destItem, opts); - - startCopy(destStat, srcItem, destItem, opts); + getStats(destStat, srcItem, destItem, opts); } } finally { dir.closeSync(); From ec30bf05e4678ae2e3d2e9768e6ed340e857d7ae Mon Sep 17 00:00:00 2001 From: Tho Date: Sat, 1 Oct 2022 18:57:38 +0800 Subject: [PATCH 2/3] fs: add test for cpSync --- test/parallel/test-fs-cp.mjs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/parallel/test-fs-cp.mjs b/test/parallel/test-fs-cp.mjs index 7181bb636699b0..3cb32787549728 100644 --- a/test/parallel/test-fs-cp.mjs +++ b/test/parallel/test-fs-cp.mjs @@ -336,6 +336,24 @@ if (!isWindows) { }, { code: 'ERR_INVALID_RETURN_VALUE' }); } +// It should not throw exception if child folder +// does not pass filter function +{ + // Create a file in dest with the same name as a child folder in src + // expect: this shouldn't throw error since filtered out by filter function + const src = nextdir(); + mkdirSync(join(src, 'foo'), mustNotMutateObjectDeep({ recursive: true })); + + const dest = nextdir(); + mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true })); + writeFileSync(join(dest, 'foo'), 'foo-content', mustNotMutateObjectDeep({ mode: 0o444 })); + + cpSync(src, dest, { + filter: (path) => !path.includes('foo'), + recursive: true, + }); +} + // It throws error if errorOnExist is true, force is false, and file or folder // copied over. { From 9382365c0e3f5e4dbef335fd9481cb130d6af9b8 Mon Sep 17 00:00:00 2001 From: Tho Date: Thu, 6 Oct 2022 08:37:36 +0800 Subject: [PATCH 3/3] fs: update comment in tests for copy Co-authored-by: Joyee Cheung --- test/parallel/test-fs-cp.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-cp.mjs b/test/parallel/test-fs-cp.mjs index 3cb32787549728..60939a3c718946 100644 --- a/test/parallel/test-fs-cp.mjs +++ b/test/parallel/test-fs-cp.mjs @@ -337,7 +337,7 @@ if (!isWindows) { } // It should not throw exception if child folder -// does not pass filter function +// is filtered out. { // Create a file in dest with the same name as a child folder in src // expect: this shouldn't throw error since filtered out by filter function