Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: fix opts.filter issue in cp async #44922

Merged
merged 3 commits into from Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 { skipped: true };
thoqbk marked this conversation as resolved.
Show resolved Hide resolved
}
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);
!skipped && (await getStatsForCopy(destStat, srcItem, destItem, opts));
thoqbk marked this conversation as resolved.
Show resolved Hide resolved
}
}

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);
}));
}
thoqbk marked this conversation as resolved.
Show resolved Hide resolved

// 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);
}));
}
thoqbk marked this conversation as resolved.
Show resolved Hide resolved

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