Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
thoqbk authored and danielleadams committed Oct 11, 2022
1 parent 4bdef48 commit bc00f3b
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 19 deletions.
27 changes: 8 additions & 19 deletions lib/internal/fs/cp/cp.js
Expand Up @@ -63,15 +63,16 @@ async function cpFn(src, dest, opts) {
process.emitWarning(warning, 'TimestampPrecisionWarning');
}
const stats = await checkPaths(src, dest, opts);
const { srcStat, destStat } = stats;
const { srcStat, destStat, skipped } = stats;
if (skipped) return;
await checkParentPaths(src, srcStat, dest);
if (opts.filter) {
return handleFilter(checkParentDir, destStat, src, dest, opts);
}
return checkParentDir(destStat, src, dest, opts);
}

async function checkPaths(src, dest, opts) {
if (opts.filter && !(await opts.filter(src, dest))) {
return { __proto__: null, skipped: true };
}
const { 0: srcStat, 1: destStat } = await getStats(src, dest, opts);
if (destStat) {
if (areIdentical(srcStat, destStat)) {
Expand Down Expand Up @@ -114,7 +115,7 @@ async function checkPaths(src, dest, opts) {
code: 'EINVAL',
});
}
return { srcStat, destStat };
return { srcStat, destStat, skipped: false };
}

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

async function handleFilter(onInclude, destStat, src, dest, opts, cb) {
const include = await opts.filter(src, dest);
if (include) return onInclude(destStat, src, dest, opts, cb);
}

function startCopy(destStat, src, dest, opts) {
if (opts.filter) {
return handleFilter(getStatsForCopy, destStat, src, dest, opts);
}
return getStatsForCopy(destStat, src, dest, opts);
}

async function getStatsForCopy(destStat, src, dest, opts) {
const statFn = opts.dereference ? stat : lstat;
const srcStat = await statFn(src);
Expand Down Expand Up @@ -328,8 +317,8 @@ async function copyDir(src, dest, opts) {
for await (const { name } of dir) {
const srcItem = join(src, name);
const destItem = join(dest, name);
const { destStat } = await checkPaths(srcItem, destItem, opts);
await startCopy(destStat, srcItem, destItem, opts);
const { destStat, skipped } = await checkPaths(srcItem, destItem, opts);
if (!skipped) await getStatsForCopy(destStat, srcItem, destItem, opts);
}
}

Expand Down
37 changes: 37 additions & 0 deletions test/parallel/test-fs-cp.mjs
Expand Up @@ -746,6 +746,43 @@ if (!isWindows) {
}));
}

// Copy async should not throw exception if child folder is filtered out.
{
const src = nextdir();
mkdirSync(join(src, 'test-cp-async'), mustNotMutateObjectDeep({ recursive: true }));

const dest = nextdir();
mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
writeFileSync(join(dest, 'test-cp-async'), 'test-content', mustNotMutateObjectDeep({ mode: 0o444 }));

cp(src, dest, {
filter: (path) => !path.includes('test-cp-async'),
recursive: true,
}, mustCall((err) => {
assert.strictEqual(err, null);
}));
}

// Copy async should not throw exception if dest is invalid but filtered out.
{
// Create dest as a file.
// Expect: cp skips the copy logic entirely and won't throw any exception in path validation process.
const src = join(nextdir(), 'bar');
mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));

const destParent = nextdir();
const dest = join(destParent, 'bar');
mkdirSync(destParent, mustNotMutateObjectDeep({ recursive: true }));
writeFileSync(dest, 'test-content', mustNotMutateObjectDeep({ mode: 0o444 }));

cp(src, dest, {
filter: (path) => !path.includes('bar'),
recursive: true,
}, mustCall((err) => {
assert.strictEqual(err, null);
}));
}

// It throws if options is not object.
{
assert.throws(
Expand Down

0 comments on commit bc00f3b

Please sign in to comment.