From 5f57697619675f8301c73a0902fe36a1eacf04c3 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 3 Mar 2026 15:31:07 -0800 Subject: [PATCH] mimic node's intended behavior with weird paths When removing any path that has `..` or `.` as path portions, call `path.normalize` on the path before attempting to process it. When removing the path `''`, throw a `stat ENOENT` error. When removing `.`, replace with `process.cwd()` and proceed as normal. Also, add support for deleting `file:` URLs and Buffer paths, which are normalized to `string` for the benefit of older Node versions. Glob patterns must still be normal `string` values. This mirrors the behavior of node, once nodejs/node#61968 lands. Closes: #342 Fixes: #326 Re: https://github.com/nodejs/node/issues/61958 Credit: @abhu85, @RajeshKumar11, @isaacs --- CHANGELOG.md | 7 +++++ README.md | 5 ++++ src/index.ts | 49 ++++++++++++++++++++--------------- src/is-strings.ts | 14 ++++++++++ src/path-arg.ts | 55 +++++++++++++++++++++++++++++---------- src/rimraf-native.ts | 2 ++ test/index.ts | 33 ++++++++++++------------ test/is-strings.ts | 11 ++++++++ test/path-arg.ts | 41 ++++++++++++++++++++--------- test/rm-cwd.ts | 61 ++++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 216 insertions(+), 62 deletions(-) create mode 100644 src/is-strings.ts create mode 100644 test/is-strings.ts create mode 100644 test/rm-cwd.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ff426c..8e3e939b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# 6.2 + +- Handle `''`, `'..'` and `'.'` paths the same as Node's intended + behavior. +- Add support for `file:` URL objects and Buffers. (Glob patterns + still have to be normal `string` types.) + # 6.1 - Move to native `fs/promises` usage instead of promisifying diff --git a/README.md b/README.md index 2e534762..5d9749ce 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,11 @@ something was omitted from the removal via a `filter` option. This first parameter is a path or array of paths. The second argument is an options object. +If interpreted as `glob` patterns, then the paths must be +normal `string` values. If not glob pattern matching, then you +may also pass in `file:` URL objects, or `Buffer` objects, which +will be turned into string paths in the normal ways. + Options: - `preserveRoot`: If set to boolean `false`, then allow the diff --git a/src/index.ts b/src/index.ts index 30068ed2..81210887 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,11 +1,8 @@ import { glob, globSync } from 'glob' -import { - optArg, - optArgSync, - RimrafAsyncOptions, - RimrafSyncOptions, -} from './opt-arg.js' -import pathArg from './path-arg.js' +import { optArg, optArgSync } from './opt-arg.js' +import type { RimrafAsyncOptions, RimrafSyncOptions } from './opt-arg.js' +import { pathArg } from './path-arg.js' +import type { PathLike } from './path-arg.js' import { rimrafManual, rimrafManualSync } from './rimraf-manual.js' import { rimrafMoveRemove, @@ -15,23 +12,27 @@ import { rimrafNative, rimrafNativeSync } from './rimraf-native.js' import { rimrafPosix, rimrafPosixSync } from './rimraf-posix.js' import { rimrafWindows, rimrafWindowsSync } from './rimraf-windows.js' import { useNative, useNativeSync } from './use-native.js' +import { isStrings } from './is-strings.js' -export { - assertRimrafOptions, - isRimrafOptions, - type RimrafAsyncOptions, - type RimrafOptions, - type RimrafSyncOptions, +export type { PathLike } from './path-arg.js' + +export type { + RimrafAsyncOptions, + RimrafOptions, + RimrafSyncOptions, } from './opt-arg.js' -const wrap = - (fn: (p: string, o: RimrafAsyncOptions) => Promise) => - async ( +export { assertRimrafOptions, isRimrafOptions } from './opt-arg.js' + +const wrap = ( + fn: (p: string, o: RimrafAsyncOptions) => Promise, +) => { + const rimraf = async ( path: string | string[], opt?: RimrafAsyncOptions, ): Promise => { const options = optArg(opt) - if (options.glob) { + if (options.glob && isStrings(path)) { path = await glob(path, options.glob) } if (Array.isArray(path)) { @@ -42,12 +43,16 @@ const wrap = return !!(await fn(pathArg(path, options), options)) } } + return rimraf +} -const wrapSync = - (fn: (p: string, o: RimrafSyncOptions) => boolean) => - (path: string | string[], opt?: RimrafSyncOptions): boolean => { +const wrapSync = (fn: (p: string, o: RimrafSyncOptions) => boolean) => { + const rimraf = ( + path: PathLike | PathLike[], + opt?: RimrafSyncOptions, + ): boolean => { const options = optArgSync(opt) - if (options.glob) { + if (options.glob && isStrings(path)) { path = globSync(path, options.glob) } if (Array.isArray(path)) { @@ -58,6 +63,8 @@ const wrapSync = return !!fn(pathArg(path, options), options) } } + return rimraf +} export const nativeSync = wrapSync(rimrafNativeSync) export const native = Object.assign(wrap(rimrafNative), { diff --git a/src/is-strings.ts b/src/is-strings.ts new file mode 100644 index 00000000..0c83bb8c --- /dev/null +++ b/src/is-strings.ts @@ -0,0 +1,14 @@ +import { PathLike } from './path-arg.js' + +export const isStrings = ( + p: PathLike | PathLike[], +): p is string | string[] => { + if (typeof p === 'string') return true + if (!Array.isArray(p)) return false + for (const s of p) { + if (typeof s !== 'string') { + return false + } + } + return true +} diff --git a/src/path-arg.ts b/src/path-arg.ts index 2972ef39..128137ae 100644 --- a/src/path-arg.ts +++ b/src/path-arg.ts @@ -1,23 +1,43 @@ -import { parse, resolve } from 'path' +import { parse, resolve, normalize } from 'path' import { inspect } from 'util' import { RimrafAsyncOptions } from './index.js' +import { fileURLToPath } from 'url' -const pathArg = (path: string, opt: RimrafAsyncOptions = {}) => { - const type = typeof path - if (type !== 'string') { +const dotPattern = /(?:^|\\|\/)\.\.?(?:$|\\|\/)/ +const BufferToString = (b: ArrayBufferView) => + Buffer.prototype.toString.call(b, 'utf8') + +export type PathLike = string | URL | ArrayBufferLike | Buffer + +export function pathArg( + path: PathLike, + opt: RimrafAsyncOptions = {}, +): string { + if (ArrayBuffer.isView(path)) { + path = BufferToString(path) + } else if (path instanceof URL && path.protocol === 'file:') { + path = fileURLToPath(path) + } + if (typeof path !== 'string') { + const type = typeof path const ctor = path && type === 'object' && path.constructor const received = - ctor && ctor.name ? `an instance of ${ctor.name}` + path instanceof URL ? `"${path.protocol}" URL object` + : ctor && ctor.name ? `an instance of ${ctor.name}` : type === 'object' ? inspect(path) : `type ${type} ${path}` const msg = - 'The "path" argument must be of type string. ' + + 'The "path" argument must be of type string, Buffer, or "file:" URL. ' + `Received ${received}` throw Object.assign(new TypeError(msg), { path, code: 'ERR_INVALID_ARG_TYPE', }) } + if (dotPattern.test(path)) { + path = normalize(path) + } + if (path === '.') path = process.cwd() if (/\0/.test(path)) { // simulate same failure that node raises @@ -28,10 +48,22 @@ const pathArg = (path: string, opt: RimrafAsyncOptions = {}) => { }) } - path = resolve(path) - const { root } = parse(path) + if (path === '') { + throw Object.assign( + new Error("'ENOENT: no such file or directory, lstat ''"), + { + errno: -2, + code: 'ENOENT', + syscall: 'lstat', + path: '', + }, + ) + } + + const rpath = resolve(path) + const { root } = parse(rpath) - if (path === root && opt.preserveRoot !== false) { + if (rpath === root && opt.preserveRoot !== false) { const msg = 'refusing to remove root directory without preserveRoot:false' throw Object.assign(new Error(msg), { @@ -42,8 +74,7 @@ const pathArg = (path: string, opt: RimrafAsyncOptions = {}) => { if (process.platform === 'win32') { const badWinChars = /[*|"<>?:]/ - const { root } = parse(path) - if (badWinChars.test(path.substring(root.length))) { + if (badWinChars.test(rpath.substring(root.length))) { throw Object.assign(new Error('Illegal characters in path.'), { path, code: 'EINVAL', @@ -53,5 +84,3 @@ const pathArg = (path: string, opt: RimrafAsyncOptions = {}) => { return path } - -export default pathArg diff --git a/src/rimraf-native.ts b/src/rimraf-native.ts index b41051b5..27d9b96c 100644 --- a/src/rimraf-native.ts +++ b/src/rimraf-native.ts @@ -2,6 +2,8 @@ import { RimrafAsyncOptions, RimrafSyncOptions } from './index.js' import { promises, rmSync } from './fs.js' const { rm } = promises +// NB: node will raise the "no rm cwd" error for us + export const rimrafNative = async ( path: string, opt: RimrafAsyncOptions, diff --git a/test/index.ts b/test/index.ts index 964a8cdf..8f64c239 100644 --- a/test/index.ts +++ b/test/index.ts @@ -1,5 +1,4 @@ import { statSync } from 'fs' -import { resolve } from 'path' import t from 'tap' import { rimraf, @@ -26,9 +25,11 @@ t.test('mocky unit tests to select the correct function', async t => { return USE_NATIVE }, }, - '../dist/esm/path-arg.js': (path: string) => { - CALLS.push(['pathArg', path]) - return path + '../dist/esm/path-arg.js': { + pathArg: (path: string) => { + CALLS.push(['pathArg', path]) + return path + }, }, '../dist/esm/opt-arg.js': { ...OPTARG, @@ -181,12 +182,12 @@ t.test('accept array of paths as first arg', async t => { true, ) t.same(ASYNC_CALLS, [ - [resolve('a'), {}], - [resolve('b'), {}], - [resolve('c'), {}], - [resolve('i'), { x: 'ya' }], - [resolve('j'), { x: 'ya' }], - [resolve('k'), { x: 'ya' }], + ['a', {}], + ['b', {}], + ['c', {}], + ['i', { x: 'ya' }], + ['j', { x: 'ya' }], + ['k', { x: 'ya' }], ]) t.equal(rimrafSync(['x', 'y', 'z']), true) @@ -197,12 +198,12 @@ t.test('accept array of paths as first arg', async t => { true, ) t.same(SYNC_CALLS, [ - [resolve('x'), {}], - [resolve('y'), {}], - [resolve('z'), {}], - [resolve('m'), { cat: 'chai' }], - [resolve('n'), { cat: 'chai' }], - [resolve('o'), { cat: 'chai' }], + ['x', {}], + ['y', {}], + ['z', {}], + ['m', { cat: 'chai' }], + ['n', { cat: 'chai' }], + ['o', { cat: 'chai' }], ]) }) diff --git a/test/is-strings.ts b/test/is-strings.ts new file mode 100644 index 00000000..1288e20d --- /dev/null +++ b/test/is-strings.ts @@ -0,0 +1,11 @@ +import t from 'tap' + +import { isStrings } from '../src/is-strings.js' + +t.equal(isStrings('asdf'), true) +t.equal(isStrings('asdf'.split('')), true) +t.equal(isStrings([]), true) +//@ts-expect-error +t.equal(isStrings([{x:1}]), false) +//@ts-expect-error +t.equal(isStrings({x:1}), false) diff --git a/test/path-arg.ts b/test/path-arg.ts index 47ff9cfc..8a1ec1c8 100644 --- a/test/path-arg.ts +++ b/test/path-arg.ts @@ -1,19 +1,18 @@ import * as PATH from 'path' import t from 'tap' +import { pathToFileURL } from 'url' import { inspect } from 'util' for (const platform of ['win32', 'posix'] as const) { t.test(platform, async t => { t.intercept(process, 'platform', { value: platform }) const path = PATH[platform] || PATH - const { default: pathArg } = (await t.mockImport( - '../src/path-arg.js', - { - path, - }, - )) as typeof import('../src/path-arg.js') + const sep = path.sep + const { pathArg } = (await t.mockImport('../src/path-arg.js', { + path, + })) as typeof import('../src/path-arg.js') - t.equal(pathArg('a/b/c'), path.resolve('a/b/c')) + t.equal(pathArg('a/b/c'), 'a/b/c') t.throws( () => pathArg('a\0b'), Error('path must be a string without null bytes'), @@ -42,23 +41,41 @@ for (const platform of ['win32', 'posix'] as const) { t.throws(() => pathArg('/', { preserveRoot: undefined }), { code: 'ERR_PRESERVE_ROOT', }) - t.equal(pathArg('/', { preserveRoot: false }), path.resolve('/')) + t.equal(pathArg('/', { preserveRoot: false }), '/') //@ts-expect-error t.throws(() => pathArg({}), { code: 'ERR_INVALID_ARG_TYPE', path: {}, message: - 'The "path" argument must be of type string. ' + + 'The "path" argument must be of type string, Buffer, or "file:" URL. ' + 'Received an instance of Object', name: 'TypeError', }) + t.equal(pathArg(pathToFileURL(process.cwd())), process.cwd()) + t.equal(pathArg('.'), process.cwd()) + t.equal(pathArg(Buffer.from('a/b/c/../.')), `a${sep}b`) + t.throws(() => pathArg(''), { + message: "'ENOENT: no such file or directory, lstat ''", + errno: -2, + code: 'ENOENT', + syscall: 'lstat', + path: '', + }) + t.throws(() => pathArg(new URL('https://example.com/')), { + code: 'ERR_INVALID_ARG_TYPE', + path: {}, + message: + 'The "path" argument must be of type string, Buffer, or "file:" URL. ' + + `Received "https:" URL`, + name: 'TypeError', + }) //@ts-expect-error t.throws(() => pathArg([]), { code: 'ERR_INVALID_ARG_TYPE', path: [], message: - 'The "path" argument must be of type string. ' + + 'The "path" argument must be of type string, Buffer, or "file:" URL. ' + 'Received an instance of Array', name: 'TypeError', }) @@ -67,7 +84,7 @@ for (const platform of ['win32', 'posix'] as const) { code: 'ERR_INVALID_ARG_TYPE', path: Object.create(null) as object, message: - 'The "path" argument must be of type string. ' + + 'The "path" argument must be of type string, Buffer, or "file:" URL. ' + `Received ${inspect(Object.create(null))}`, name: 'TypeError', }) @@ -76,7 +93,7 @@ for (const platform of ['win32', 'posix'] as const) { code: 'ERR_INVALID_ARG_TYPE', path: true, message: - 'The "path" argument must be of type string. ' + + 'The "path" argument must be of type string, Buffer, or "file:" URL. ' + `Received type boolean true`, name: 'TypeError', }) diff --git a/test/rm-cwd.ts b/test/rm-cwd.ts new file mode 100644 index 00000000..c7020946 --- /dev/null +++ b/test/rm-cwd.ts @@ -0,0 +1,61 @@ +import t from 'tap' +import { + rimraf, + rimrafSync, + manual, + manualSync, + native, + nativeSync, + posix, + posixSync, + windows, + windowsSync, + moveRemove, + moveRemoveSync, +} from '../src/index.js' +import { statSync } from 'node:fs' + +for (const [name, impl] of Object.entries({ + rimraf, + manual, + native, + posix, + windows, + moveRemove, +})) { + t.test(name, async t => { + t.chdir(t.testdir({ a: { b: { c: { d: { e: '' } } } } })) + t.equal(statSync('a/b/c/d/e').isFile(), true) + await impl('a/b/c/../.') + t.throws(() => statSync('a/b')) + await impl('a/.') + t.throws(() => statSync('a')) + await t.rejects(impl('')) + t.equal(statSync(t.testdirName).isDirectory(), true) + await impl('.') + t.throws(() => statSync(t.testdirName)) + }) +} + +for (const [name, impl] of Object.entries({ + rimrafSync, + manualSync, + nativeSync, + posixSync, + windowsSync, + moveRemoveSync, +})) { + t.test(name, t => { + t.chdir(t.testdir({ a: { b: { c: { d: { e: '' } } } } })) + t.equal(statSync('a/b/c/d/e').isFile(), true) + impl('a/b/c/../.') + t.throws(() => statSync('a/b')) + impl('a/.') + t.throws(() => statSync('a')) + t.throws(() => impl('')) + t.equal(statSync(t.testdirName).isDirectory(), true) + impl('.') + t.throws(() => statSync(t.testdirName)) + t.end() + }) +}