FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: fix opts.filter issue in cpSync · nodejs/node@670def3 · GitHub

/ node Public

Commit 670def3

Browse files
thoqbk
authored and
RafaelGSS
committed
fs: fix opts.filter issue in cpSync
PR-URL: #45143 Fixes: #44720 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 3369120 commit 670def3

2 files changed

Lines changed: 27 additions & 28 deletions

File tree

‎lib/internal/fs/cp/cp-sync.js‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,20 @@ function cpSyncFn(src, dest, opts) {
5555
'node is not recommended';
5656
process.emitWarning(warning, 'TimestampPrecisionWarning');
5757
}
58-
const { srcStat, destStat } = checkPathsSync(src, dest, opts);
58+
const { srcStat, destStat, skipped } = checkPathsSync(src, dest, opts);
59+
if (skipped) return;
5960
checkParentPathsSync(src, srcStat, dest);
60-
return handleFilterAndCopy(destStat, src, dest, opts);
61+
return checkParentDir(destStat, src, dest, opts);
6162
}
6263

6364
function checkPathsSync(src, dest, opts) {
65+
if (opts.filter) {
66+
const shouldCopy = opts.filter(src, dest);
67+
if (isPromise(shouldCopy)) {
68+
throw new ERR_INVALID_RETURN_VALUE('boolean', 'filter', shouldCopy);
69+
}
70+
if (!shouldCopy) return { __proto__: null, skipped: true };
71+
}
6472
const { srcStat, destStat } = getStatsSync(src, dest, opts);
6573

6674
if (destStat) {
@@ -104,7 +112,7 @@ function checkPathsSync(src, dest, opts) {
104112
code: 'EINVAL',
105113
});
106114
}
107-
return { srcStat, destStat };
115+
return { __proto__: null, srcStat, destStat, skipped: false };
108116
}
109117

110118
function getStatsSync(src, dest, opts) {
@@ -145,24 +153,12 @@ function checkParentPathsSync(src, srcStat, dest) {
145153
return checkParentPathsSync(src, srcStat, destParent);
146154
}
147155

148-
function handleFilterAndCopy(destStat, src, dest, opts) {
149-
if (opts.filter) {
150-
const shouldCopy = opts.filter(src, dest);
151-
if (isPromise(shouldCopy)) {
152-
throw new ERR_INVALID_RETURN_VALUE('boolean', 'filter', shouldCopy);
153-
}
154-
if (!shouldCopy) return;
155-
}
156+
function checkParentDir(destStat, src, dest, opts) {
156157
const destParent = dirname(dest);
157158
if (!existsSync(destParent)) mkdirSync(destParent, { recursive: true });
158159
return getStats(destStat, src, dest, opts);
159160
}
160161

161-
function startCopy(destStat, src, dest, opts) {
162-
if (opts.filter && !opts.filter(src, dest)) return;
163-
return getStats(destStat, src, dest, opts);
164-
}
165-
166162
function getStats(destStat, src, dest, opts) {
167163
const statSyncFn = opts.dereference ? statSync : lstatSync;
168164
const srcStat = statSyncFn(src);
@@ -284,9 +280,8 @@ function copyDir(src, dest, opts) {
284280
const { name } = dirent;
285281
const srcItem = join(src, name);
286282
const destItem = join(dest, name);
287-
const { destStat } = checkPathsSync(srcItem, destItem, opts);
288-
289-
startCopy(destStat, srcItem, destItem, opts);
283+
const { destStat, skipped } = checkPathsSync(srcItem, destItem, opts);
284+
if (!skipped) getStats(destStat, srcItem, destItem, opts);
290285
}
291286
} finally {
292287
dir.closeSync();

‎test/parallel/test-fs-cp.mjs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -746,24 +746,26 @@ if (!isWindows) {
746746
}));
747747
}
748748

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

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

758-
cp(src, dest, {
759-
filter: (path) => !path.includes('test-cp-async'),
758+
const opts = {
759+
filter: (path) => !path.includes('test-cp'),
760760
recursive: true,
761-
}, mustCall((err) => {
761+
};
762+
cp(src, dest, opts, mustCall((err) => {
762763
assert.strictEqual(err, null);
763764
}));
765+
cpSync(src, dest, opts);
764766
}
765767

766-
// Copy async should not throw exception if dest is invalid but filtered out.
768+
// Copy should not throw exception if dest is invalid but filtered out.
767769
{
768770
// Create dest as a file.
769771
// Expect: cp skips the copy logic entirely and won't throw any exception in path validation process.
@@ -775,12 +777,14 @@ if (!isWindows) {
775777
mkdirSync(destParent, mustNotMutateObjectDeep({ recursive: true }));
776778
writeFileSync(dest, 'test-content', mustNotMutateObjectDeep({ mode: 0o444 }));
777779

778-
cp(src, dest, {
780+
const opts = {
779781
filter: (path) => !path.includes('bar'),
780782
recursive: true,
781-
}, mustCall((err) => {
783+
};
784+
cp(src, dest, opts, mustCall((err) => {
782785
assert.strictEqual(err, null);
783786
}));
787+
cpSync(src, dest, opts);
784788
}
785789

786790
// It throws if options is not object.

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL