diff --git a/lib/fs.js b/lib/fs.js index 4436fa2df6e6..5e73ba6fb585 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1537,6 +1537,29 @@ function rmdirSync(path, options) { binding.rmdir(path); } +/** + * Asynchronously removes files and + * directories (modeled on the standard POSIX `rm` utility). + * @param {string | Buffer | URL} path + * @param {{ + * force?: boolean; + * maxRetries?: number; + * recursive?: boolean; + * retryDelay?: number; + * }} [options] +function normalizePathForRm(path) { + if (typeof path === 'string') { + return pathModule.normalize(path); + } + if (isArrayBufferView(path)) { + const buf = Buffer.from(path.buffer, path.byteOffset, path.byteLength); + const str = BufferToString(buf, 'latin1'); + const normalized = pathModule.normalize(str); + return Buffer.from(normalized, 'latin1'); + } + return path; +} + /** * Asynchronously removes files and * directories (modeled on the standard POSIX `rm` utility). @@ -1560,6 +1583,7 @@ function rm(path, options, callback) { if (h !== null && vfsVoid(h.rm(path, options), callback)) return; path = getValidatedPath(path); + path = normalizePathForRm(path); validateRmOptions(path, options, false, (err, options) => { if (err) { @@ -1588,8 +1612,10 @@ function rmSync(path, options) { const result = h.rmSync(path, options); if (result !== undefined) return; } + path = getValidatedPath(path); + path = normalizePathForRm(path); const opts = validateRmOptionsSync(path, options, false); - return binding.rmSync(getValidatedPath(path), opts.maxRetries, opts.recursive, opts.retryDelay); + return binding.rmSync(path, opts.maxRetries, opts.recursive, opts.retryDelay); } /** diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index efa981c55e31..2c7ea4d21eed 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1600,6 +1600,19 @@ async function ftruncate(handle, len = 0) { ); } +function normalizePathForRm(path) { + if (typeof path === 'string') { + return pathModule.normalize(path); + } + if (isArrayBufferView(path)) { + const buf = Buffer.from(path.buffer, path.byteOffset, path.byteLength); + const str = BufferToString(buf, 'latin1'); + const normalized = pathModule.normalize(str); + return Buffer.from(normalized, 'latin1'); + } + return path; +} + async function rm(path, options) { const h = vfsState.handlers; if (h !== null) { @@ -1607,6 +1620,7 @@ async function rm(path, options) { if (promise !== undefined) { await promise; return; } } path = getValidatedPath(path); + path = normalizePathForRm(path); options = await validateRmOptionsPromise(path, options, false); return lazyRimRaf()(path, options); } diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 80600d114b9a..d4badfd09514 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -657,3 +657,89 @@ if (isGitPresent) { // Clean up parent directory fs.rmSync(dirname, { recursive: true, force: true }); } + +// Test that rm/rmSync normalize '.' and '..' in paths before processing. +// Regression test for https://github.com/nodejs/node/issues/61958 +{ + const cases = [ + { segments: ['a', 'b', '..', '.'], target: 'a' }, + { segments: ['a', 'b', '..'], target: 'a' }, + { segments: ['a', '.'], target: 'a' }, + { segments: ['a', 'b', 'c', '.'], target: path.join('a', 'b', 'c') }, + { segments: ['a', 'b', 'c', '..', '..'], target: 'a' }, + { segments: ['a', 'b', 'c', 'd', '..', '..', '..'], target: 'a' }, + { segments: ['a', 'b', '..', '.', 'b'], target: path.join('a', 'b') }, + { segments: ['.', 'a'], target: 'a' }, + ]; + + for (const { segments, target } of cases) { + // rmSync with string path + { + const base = nextDirPath('rm-dot-sync-str'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, ...segments].join(path.sep); + fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true, force: true })); + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + } + + // rmSync with Buffer path + { + const base = nextDirPath('rm-dot-sync-buf'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = Buffer.from([base, ...segments].join(path.sep)); + fs.rmSync(weirdPath, common.mustNotMutateObjectDeep({ recursive: true, force: true })); + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + } + + // fs.rm (callback) with string path + { + const base = nextDirPath('rm-dot-cb-str'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, ...segments].join(path.sep); + fs.rm(weirdPath, + common.mustNotMutateObjectDeep({ recursive: true, force: true }), + common.mustSucceed(() => { + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + })); + } + + // fs.promises.rm with string path + { + const base = nextDirPath('rm-dot-prom-str'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = [base, ...segments].join(path.sep); + fs.promises.rm(weirdPath, + common.mustNotMutateObjectDeep({ recursive: true, force: true })) + .then(common.mustCall(() => { + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + })); + } + + // fs.promises.rm with Buffer path + { + const base = nextDirPath('rm-dot-prom-buf'); + fs.mkdirSync(path.join(base, 'a', 'b', 'c', 'd'), + common.mustNotMutateObjectDeep({ recursive: true })); + const weirdPath = Buffer.from([base, ...segments].join(path.sep)); + fs.promises.rm(weirdPath, + common.mustNotMutateObjectDeep({ recursive: true, force: true })) + .then(common.mustCall(() => { + assert.strictEqual(fs.existsSync(path.join(base, target)), false); + })); + } + } + + // Non-UTF-8 Buffer path byte preservation test + { + const base = nextDirPath('rm-dot-buf-latin1'); + fs.mkdirSync(path.join(base, 'a', 'b'), + common.mustNotMutateObjectDeep({ recursive: true })); + const bufPath = Buffer.from([base, 'a', 'b', '..', '.'].join(path.sep)); + fs.rmSync(bufPath, common.mustNotMutateObjectDeep({ recursive: true, force: true })); + assert.strictEqual(fs.existsSync(path.join(base, 'a')), false); + } +}