Skip to content

Commit 0ee0e2e

Browse files
committed
fs: normalize paths with dot segments in rm and rmSync
When paths containing '.' or '..' components (e.g., 'a/b/../.') are passed to fs.rm(), fs.rmSync(), or fs.promises.rm(), the different codepaths (native binding.rmSync vs JS rimraf) could produce inconsistent results because the path was used as-is without normalization. Add pathModule.normalize() after getValidatedPath() in all three rm variants to ensure dot-segment components are resolved before the path reaches the underlying removal implementation. Fixes: #61958 Signed-off-by: Rohith Pariki <rohithpariki@gmail.com>
1 parent 7fab656 commit 0ee0e2e

3 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎lib/fs.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1560,6 +1560,7 @@ function rm(path, options, callback) {
15601560
if (h !== null && vfsVoid(h.rm(path, options), callback)) return;
15611561

15621562
path = getValidatedPath(path);
1563+
path = pathModule.normalize(path);
15631564

15641565
validateRmOptions(path, options, false, (err, options) => {
15651566
if (err) {
@@ -1588,8 +1589,10 @@ function rmSync(path, options) {
15881589
const result = h.rmSync(path, options);
15891590
if (result !== undefined) return;
15901591
}
1592+
path = getValidatedPath(path);
1593+
path = pathModule.normalize(path);
15911594
const opts = validateRmOptionsSync(path, options, false);
1592-
return binding.rmSync(getValidatedPath(path), opts.maxRetries, opts.recursive, opts.retryDelay);
1595+
return binding.rmSync(path, opts.maxRetries, opts.recursive, opts.retryDelay);
15931596
}
15941597

15951598
/**

‎lib/internal/fs/promises.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,7 @@ async function rm(path, options) {
16071607
if (promise !== undefined) { await promise; return; }
16081608
}
16091609
path = getValidatedPath(path);
1610+
path = pathModule.normalize(path);
16101611
options = await validateRmOptionsPromise(path, options, false);
16111612
return lazyRimRaf()(path, options);
16121613
}

‎test/parallel/test-fs-rm.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,3 +657,45 @@ if (isGitPresent) {
657657
// Clean up parent directory
658658
fs.rmSync(dirname, { recursive: true, force: true });
659659
}
660+
661+
// Test that rm/rmSync normalize '.' and '..' in paths before processing.
662+
// Regression test for https://github.com/nodejs/node/issues/61958
663+
//
664+
// Each variant gets its own base directory to avoid async/sync races.
665+
// The path is constructed by joining components with path.sep so that
666+
// the '..' and '.' are preserved and not pre-normalized by path.join.
667+
{
668+
// --- rmSync: <base>/a/b/../. should remove <base>/a entirely ---
669+
const base = nextDirPath('dotdot-sync');
670+
fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'),
671+
common.mustNotMutateObjectDeep({ recursive: true }));
672+
const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep);
673+
fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true }));
674+
assert.strictEqual(fs.existsSync(path.join(base, 'a')), false);
675+
}
676+
677+
{
678+
// --- fs.rm (callback): same path construction ---
679+
const base = nextDirPath('dotdot-cb');
680+
fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'),
681+
common.mustNotMutateObjectDeep({ recursive: true }));
682+
const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep);
683+
fs.rm(weirdPath,
684+
common.mustNotMutateObjectDeep({ recursive: true }),
685+
common.mustSucceed(() => {
686+
assert.strictEqual(fs.existsSync(path.join(base, 'a')), false);
687+
}));
688+
}
689+
690+
{
691+
// --- fs.promises.rm: same path construction ---
692+
const base = nextDirPath('dotdot-prom');
693+
fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'),
694+
common.mustNotMutateObjectDeep({ recursive: true }));
695+
const weirdPath = [base, 'a', 'b', '..', '.'].join(path.sep);
696+
fs.promises.rm(weirdPath,
697+
common.mustNotMutateObjectDeep({ recursive: true }))
698+
.then(common.mustCall(() => {
699+
assert.strictEqual(fs.existsSync(path.join(base, 'a')), false);
700+
}));
701+
}

0 commit comments

Comments
 (0)