Skip to content

Commit bc00f3b

Browse files
thoqbkdanielleadams
authored andcommitted
fs: fix opts.filter issue in cp async
PR-URL: #44922 Fixes: #44720 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 4bdef48 commit bc00f3b

File tree

2 files changed

+45
-19
lines changed

2 files changed

+45
-19
lines changed

lib/internal/fs/cp/cp.js

+8-19
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,16 @@ async function cpFn(src, dest, opts) {
6363
process.emitWarning(warning, 'TimestampPrecisionWarning');
6464
}
6565
const stats = await checkPaths(src, dest, opts);
66-
const { srcStat, destStat } = stats;
66+
const { srcStat, destStat, skipped } = stats;
67+
if (skipped) return;
6768
await checkParentPaths(src, srcStat, dest);
68-
if (opts.filter) {
69-
return handleFilter(checkParentDir, destStat, src, dest, opts);
70-
}
7169
return checkParentDir(destStat, src, dest, opts);
7270
}
7371

7472
async function checkPaths(src, dest, opts) {
73+
if (opts.filter && !(await opts.filter(src, dest))) {
74+
return { __proto__: null, skipped: true };
75+
}
7576
const { 0: srcStat, 1: destStat } = await getStats(src, dest, opts);
7677
if (destStat) {
7778
if (areIdentical(srcStat, destStat)) {
@@ -114,7 +115,7 @@ async function checkPaths(src, dest, opts) {
114115
code: 'EINVAL',
115116
});
116117
}
117-
return { srcStat, destStat };
118+
return { srcStat, destStat, skipped: false };
118119
}
119120

120121
function areIdentical(srcStat, destStat) {
@@ -190,18 +191,6 @@ function isSrcSubdir(src, dest) {
190191
return ArrayPrototypeEvery(srcArr, (cur, i) => destArr[i] === cur);
191192
}
192193

193-
async function handleFilter(onInclude, destStat, src, dest, opts, cb) {
194-
const include = await opts.filter(src, dest);
195-
if (include) return onInclude(destStat, src, dest, opts, cb);
196-
}
197-
198-
function startCopy(destStat, src, dest, opts) {
199-
if (opts.filter) {
200-
return handleFilter(getStatsForCopy, destStat, src, dest, opts);
201-
}
202-
return getStatsForCopy(destStat, src, dest, opts);
203-
}
204-
205194
async function getStatsForCopy(destStat, src, dest, opts) {
206195
const statFn = opts.dereference ? stat : lstat;
207196
const srcStat = await statFn(src);
@@ -328,8 +317,8 @@ async function copyDir(src, dest, opts) {
328317
for await (const { name } of dir) {
329318
const srcItem = join(src, name);
330319
const destItem = join(dest, name);
331-
const { destStat } = await checkPaths(srcItem, destItem, opts);
332-
await startCopy(destStat, srcItem, destItem, opts);
320+
const { destStat, skipped } = await checkPaths(srcItem, destItem, opts);
321+
if (!skipped) await getStatsForCopy(destStat, srcItem, destItem, opts);
333322
}
334323
}
335324

test/parallel/test-fs-cp.mjs

+37
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,43 @@ if (!isWindows) {
746746
}));
747747
}
748748

749+
// Copy async should not throw exception if child folder is filtered out.
750+
{
751+
const src = nextdir();
752+
mkdirSync(join(src, 'test-cp-async'), mustNotMutateObjectDeep({ recursive: true }));
753+
754+
const dest = nextdir();
755+
mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
756+
writeFileSync(join(dest, 'test-cp-async'), 'test-content', mustNotMutateObjectDeep({ mode: 0o444 }));
757+
758+
cp(src, dest, {
759+
filter: (path) => !path.includes('test-cp-async'),
760+
recursive: true,
761+
}, mustCall((err) => {
762+
assert.strictEqual(err, null);
763+
}));
764+
}
765+
766+
// Copy async should not throw exception if dest is invalid but filtered out.
767+
{
768+
// Create dest as a file.
769+
// Expect: cp skips the copy logic entirely and won't throw any exception in path validation process.
770+
const src = join(nextdir(), 'bar');
771+
mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
772+
773+
const destParent = nextdir();
774+
const dest = join(destParent, 'bar');
775+
mkdirSync(destParent, mustNotMutateObjectDeep({ recursive: true }));
776+
writeFileSync(dest, 'test-content', mustNotMutateObjectDeep({ mode: 0o444 }));
777+
778+
cp(src, dest, {
779+
filter: (path) => !path.includes('bar'),
780+
recursive: true,
781+
}, mustCall((err) => {
782+
assert.strictEqual(err, null);
783+
}));
784+
}
785+
749786
// It throws if options is not object.
750787
{
751788
assert.throws(

0 commit comments

Comments
 (0)