fs: remove permissive rmdir recursive

PR-URL: https://github.com/nodejs/node/pull/37216
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: Ian Sutherland <ian@iansutherland.ca>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This commit is contained in:
Antoine du Hamel 2021-02-03 20:18:50 +01:00
parent 3cc9aec988
commit 9948036ee0
10 changed files with 167 additions and 43 deletions

View File

@ -1048,6 +1048,16 @@ Renames `oldPath` to `newPath`.
<!-- YAML <!-- YAML
added: v10.0.0 added: v10.0.0
changes: changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: "Using `fsPromises.rmdir(path, { recursive: true })` on a `path`
that is a file is no longer permitted and results in an
`ENOENT` error on Windows and an `ENOTDIR` error on POSIX."
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: "Using `fsPromises.rmdir(path, { recursive: true })` on a `path`
that does not exist is no longer permitted and results in a
`ENOENT` error."
- version: REPLACEME - version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302 pr-url: https://github.com/nodejs/node/pull/37302
description: The `recursive` option is deprecated, using it triggers a description: The `recursive` option is deprecated, using it triggers a
@ -1075,8 +1085,8 @@ changes:
represents the number of retries. This option is ignored if the `recursive` represents the number of retries. This option is ignored if the `recursive`
option is not `true`. **Default:** `0`. option is not `true`. **Default:** `0`.
* `recursive` {boolean} If `true`, perform a recursive directory removal. In * `recursive` {boolean} If `true`, perform a recursive directory removal. In
recursive mode, errors are not reported if `path` does not exist, and recursive mode, operations are retried on failure. **Default:** `false`.
operations are retried on failure. **Default:** `false`. **Deprecated**. **Deprecated**.
* `retryDelay` {integer} The amount of time in milliseconds to wait between * `retryDelay` {integer} The amount of time in milliseconds to wait between
retries. This option is ignored if the `recursive` option is not `true`. retries. This option is ignored if the `recursive` option is not `true`.
**Default:** `100`. **Default:** `100`.
@ -1088,10 +1098,8 @@ Using `fsPromises.rmdir()` on a file (not a directory) results in the
promise being rejected with an `ENOENT` error on Windows and an `ENOTDIR` promise being rejected with an `ENOENT` error on Windows and an `ENOTDIR`
error on POSIX. error on POSIX.
Setting `recursive` to `true` results in behavior similar to the Unix command To get a behavior similar to the `rm -rf` Unix command, use
`rm -rf`: an error will not be raised for paths that do not exist, and paths [`fsPromises.rm()`][] with options `{ recursive: true, force: true }`.
that represent files will be deleted. The `recursive` option is deprecated,
`ENOTDIR` and `ENOENT` will be thrown in the future.
### `fsPromises.rm(path[, options])` ### `fsPromises.rm(path[, options])`
<!-- YAML <!-- YAML
@ -3146,6 +3154,16 @@ rename('oldFile.txt', 'newFile.txt', (err) => {
<!-- YAML <!-- YAML
added: v0.0.2 added: v0.0.2
changes: changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: "Using `fs.rmdir(path, { recursive: true })` on a `path` that is
a file is no longer permitted and results in an `ENOENT` error
on Windows and an `ENOTDIR` error on POSIX."
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: "Using `fs.rmdir(path, { recursive: true })` on a `path` that
does not exist is no longer permitted and results in a `ENOENT`
error."
- version: REPLACEME - version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302 pr-url: https://github.com/nodejs/node/pull/37302
description: The `recursive` option is deprecated, using it triggers a description: The `recursive` option is deprecated, using it triggers a
@ -3185,8 +3203,8 @@ changes:
represents the number of retries. This option is ignored if the `recursive` represents the number of retries. This option is ignored if the `recursive`
option is not `true`. **Default:** `0`. option is not `true`. **Default:** `0`.
* `recursive` {boolean} If `true`, perform a recursive directory removal. In * `recursive` {boolean} If `true`, perform a recursive directory removal. In
recursive mode, errors are not reported if `path` does not exist, and recursive mode, operations are retried on failure. **Default:** `false`.
operations are retried on failure. **Default:** `false`. **Deprecated**. **Deprecated**.
* `retryDelay` {integer} The amount of time in milliseconds to wait between * `retryDelay` {integer} The amount of time in milliseconds to wait between
retries. This option is ignored if the `recursive` option is not `true`. retries. This option is ignored if the `recursive` option is not `true`.
**Default:** `100`. **Default:** `100`.
@ -3199,10 +3217,8 @@ to the completion callback.
Using `fs.rmdir()` on a file (not a directory) results in an `ENOENT` error on Using `fs.rmdir()` on a file (not a directory) results in an `ENOENT` error on
Windows and an `ENOTDIR` error on POSIX. Windows and an `ENOTDIR` error on POSIX.
Setting `recursive` to `true` results in behavior similar to the Unix command To get a behavior similar to the `rm -rf` Unix command, use [`fs.rm()`][]
`rm -rf`: an error will not be raised for paths that do not exist, and paths with options `{ recursive: true, force: true }`.
that represent files will be deleted. The `recursive` option is deprecated,
`ENOTDIR` and `ENOENT` will be thrown in the future.
### `fs.rm(path[, options], callback)` ### `fs.rm(path[, options], callback)`
<!-- YAML <!-- YAML
@ -4777,6 +4793,16 @@ See the POSIX rename(2) documentation for more details.
<!-- YAML <!-- YAML
added: v0.1.21 added: v0.1.21
changes: changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: "Using `fs.rmdirSync(path, { recursive: true })` on a `path`
that is a file is no longer permitted and results in an
`ENOENT` error on Windows and an `ENOTDIR` error on POSIX."
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37216
description: "Using `fs.rmdirSync(path, { recursive: true })` on a `path`
that does not exist is no longer permitted and results in a
`ENOENT` error."
- version: REPLACEME - version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37302 pr-url: https://github.com/nodejs/node/pull/37302
description: The `recursive` option is deprecated, using it triggers a description: The `recursive` option is deprecated, using it triggers a
@ -4808,8 +4834,8 @@ changes:
represents the number of retries. This option is ignored if the `recursive` represents the number of retries. This option is ignored if the `recursive`
option is not `true`. **Default:** `0`. option is not `true`. **Default:** `0`.
* `recursive` {boolean} If `true`, perform a recursive directory removal. In * `recursive` {boolean} If `true`, perform a recursive directory removal. In
recursive mode, errors are not reported if `path` does not exist, and recursive mode, operations are retried on failure. **Default:** `false`.
operations are retried on failure. **Default:** `false`. **Deprecated**. **Deprecated**.
* `retryDelay` {integer} The amount of time in milliseconds to wait between * `retryDelay` {integer} The amount of time in milliseconds to wait between
retries. This option is ignored if the `recursive` option is not `true`. retries. This option is ignored if the `recursive` option is not `true`.
**Default:** `100`. **Default:** `100`.
@ -4819,10 +4845,8 @@ Synchronous rmdir(2). Returns `undefined`.
Using `fs.rmdirSync()` on a file (not a directory) results in an `ENOENT` error Using `fs.rmdirSync()` on a file (not a directory) results in an `ENOENT` error
on Windows and an `ENOTDIR` error on POSIX. on Windows and an `ENOTDIR` error on POSIX.
Setting `recursive` to `true` results in behavior similar to the Unix command To get a behavior similar to the `rm -rf` Unix command, use [`fs.rmSync()`][]
`rm -rf`: an error will not be raised for paths that do not exist, and paths with options `{ recursive: true, force: true }`.
that represent files will be deleted. The `recursive` option is deprecated,
`ENOTDIR` and `ENOENT` will be thrown in the future.
### `fs.rmSync(path[, options])` ### `fs.rmSync(path[, options])`
<!-- YAML <!-- YAML
@ -6670,6 +6694,8 @@ the file contents.
[`fs.readdirSync()`]: #fs_fs_readdirsync_path_options [`fs.readdirSync()`]: #fs_fs_readdirsync_path_options
[`fs.readv()`]: #fs_fs_readv_fd_buffers_position_callback [`fs.readv()`]: #fs_fs_readv_fd_buffers_position_callback
[`fs.realpath()`]: #fs_fs_realpath_path_options_callback [`fs.realpath()`]: #fs_fs_realpath_path_options_callback
[`fs.rm()`]: #fs_fs_rm_path_options_callback
[`fs.rmSync()`]: #fs_fs_rmsync_path_options
[`fs.rmdir()`]: #fs_fs_rmdir_path_options_callback [`fs.rmdir()`]: #fs_fs_rmdir_path_options_callback
[`fs.stat()`]: #fs_fs_stat_path_options_callback [`fs.stat()`]: #fs_fs_stat_path_options_callback
[`fs.symlink()`]: #fs_fs_symlink_target_path_type_callback [`fs.symlink()`]: #fs_fs_symlink_target_path_type_callback
@ -6681,6 +6707,7 @@ the file contents.
[`fs.writev()`]: #fs_fs_writev_fd_buffers_position_callback [`fs.writev()`]: #fs_fs_writev_fd_buffers_position_callback
[`fsPromises.open()`]: #fs_fspromises_open_path_flags_mode [`fsPromises.open()`]: #fs_fspromises_open_path_flags_mode
[`fsPromises.opendir()`]: #fs_fspromises_opendir_path_options [`fsPromises.opendir()`]: #fs_fspromises_opendir_path_options
[`fsPromises.rm()`]: #fs_fspromises_rm_path_options
[`fsPromises.utimes()`]: #fs_fspromises_utimes_path_atime_mtime [`fsPromises.utimes()`]: #fs_fspromises_utimes_path_atime_mtime
[`inotify(7)`]: https://man7.org/linux/man-pages/man7/inotify.7.html [`inotify(7)`]: https://man7.org/linux/man-pages/man7/inotify.7.html
[`kqueue(2)`]: https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2 [`kqueue(2)`]: https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2

View File

@ -898,15 +898,20 @@ function rmdir(path, options, callback) {
emitRecursiveRmdirWarning(); emitRecursiveRmdirWarning();
validateRmOptions( validateRmOptions(
path, path,
{ ...options, force: true }, { ...options, force: false },
true, true,
(err, options) => { (err, options) => {
if (err === false) {
const req = new FSReqCallback();
req.oncomplete = callback;
return binding.rmdir(path, req);
}
if (err) { if (err) {
return callback(err); return callback(err);
} }
lazyLoadRimraf(); lazyLoadRimraf();
return rimraf(path, options, callback); rimraf(path, options, callback);
}); });
} else { } else {
validateRmdirOptions(options); validateRmdirOptions(options);
@ -921,12 +926,15 @@ function rmdirSync(path, options) {
if (options?.recursive) { if (options?.recursive) {
emitRecursiveRmdirWarning(); emitRecursiveRmdirWarning();
options = validateRmOptionsSync(path, { ...options, force: true }, true); options = validateRmOptionsSync(path, { ...options, force: false }, true);
if (options !== false) {
lazyLoadRimraf(); lazyLoadRimraf();
return rimrafSync(pathModule.toNamespacedPath(path), options); return rimrafSync(pathModule.toNamespacedPath(path), options);
} }
} else {
validateRmdirOptions(options); validateRmdirOptions(options);
}
const ctx = { path }; const ctx = { path };
binding.rmdir(pathModule.toNamespacedPath(path), undefined, ctx); binding.rmdir(pathModule.toNamespacedPath(path), undefined, ctx);
return handleErrorFromBinding(ctx); return handleErrorFromBinding(ctx);

View File

@ -505,8 +505,11 @@ async function rmdir(path, options) {
if (options.recursive) { if (options.recursive) {
emitRecursiveRmdirWarning(); emitRecursiveRmdirWarning();
const stats = await stat(path);
if (stats.isDirectory()) {
return rimrafPromises(path, options); return rimrafPromises(path, options);
} }
}
return binding.rmdir(path, kUsePromises); return binding.rmdir(path, kUsePromises);
} }

View File

@ -698,20 +698,24 @@ const defaultRmdirOptions = {
recursive: false, recursive: false,
}; };
const validateRmOptions = hideStackFrames((path, options, warn, callback) => { const validateRmOptions = hideStackFrames((path, options, expectDir, cb) => {
options = validateRmdirOptions(options, defaultRmOptions); options = validateRmdirOptions(options, defaultRmOptions);
validateBoolean(options.force, 'options.force'); validateBoolean(options.force, 'options.force');
lazyLoadFs().stat(path, (err, stats) => { lazyLoadFs().stat(path, (err, stats) => {
if (err) { if (err) {
if (options.force && err.code === 'ENOENT') { if (options.force && err.code === 'ENOENT') {
return callback(null, options); return cb(null, options);
} }
return callback(err, options); return cb(err, options);
}
if (expectDir && !stats.isDirectory()) {
return cb(false);
} }
if (stats.isDirectory() && !options.recursive) { if (stats.isDirectory() && !options.recursive) {
return callback(new ERR_FS_EISDIR({ return cb(new ERR_FS_EISDIR({
code: 'EISDIR', code: 'EISDIR',
message: 'is a directory', message: 'is a directory',
path, path,
@ -719,18 +723,22 @@ const validateRmOptions = hideStackFrames((path, options, warn, callback) => {
errno: EISDIR errno: EISDIR
})); }));
} }
return callback(null, options); return cb(null, options);
}); });
}); });
const validateRmOptionsSync = hideStackFrames((path, options, warn) => { const validateRmOptionsSync = hideStackFrames((path, options, expectDir) => {
options = validateRmdirOptions(options, defaultRmOptions); options = validateRmdirOptions(options, defaultRmOptions);
validateBoolean(options.force, 'options.force'); validateBoolean(options.force, 'options.force');
if (!options.force || warn || !options.recursive) { if (!options.force || expectDir || !options.recursive) {
const isDirectory = lazyLoadFs() const isDirectory = lazyLoadFs()
.statSync(path, { throwIfNoEntry: !options.force })?.isDirectory(); .statSync(path, { throwIfNoEntry: !options.force })?.isDirectory();
if (expectDir && !isDirectory) {
return false;
}
if (isDirectory && !options.recursive) { if (isDirectory && !options.recursive) {
throw new ERR_FS_EISDIR({ throw new ERR_FS_EISDIR({
code: 'EISDIR', code: 'EISDIR',

View File

@ -1,6 +1,7 @@
'use strict'; 'use strict';
const common = require('../common'); const common = require('../common');
const tmpdir = require('../common/tmpdir'); const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
@ -14,5 +15,9 @@ tmpdir.refresh();
'will be removed. Use fs.rm(path, { recursive: true }) instead', 'will be removed. Use fs.rm(path, { recursive: true }) instead',
'DEP0147' 'DEP0147'
); );
fs.rmdirSync(path.join(tmpdir.path, 'noexist.txt'), { recursive: true }); assert.throws(
() => fs.rmdirSync(path.join(tmpdir.path, 'noexist.txt'),
{ recursive: true }),
{ code: 'ENOENT' }
);
} }

View File

@ -1,13 +1,13 @@
'use strict'; 'use strict';
const common = require('../common'); const common = require('../common');
const tmpdir = require('../common/tmpdir'); const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
tmpdir.refresh(); tmpdir.refresh();
{ {
// Should warn when trying to delete a file
common.expectWarning( common.expectWarning(
'DeprecationWarning', 'DeprecationWarning',
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' + 'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
@ -16,5 +16,8 @@ tmpdir.refresh();
); );
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt'); const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
fs.writeFileSync(filePath, ''); fs.writeFileSync(filePath, '');
fs.rmdirSync(filePath, { recursive: true }); assert.throws(
() => fs.rmdirSync(filePath, { recursive: true }),
{ code: common.isWindows ? 'ENOENT' : 'ENOTDIR' }
);
} }

View File

@ -0,0 +1,36 @@
'use strict';
const common = require('../common');
const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
tmpdir.refresh();
{
assert.throws(
() =>
fs.rmdirSync(path.join(tmpdir.path, 'noexist.txt'), { recursive: true }),
{
code: 'ENOENT',
}
);
}
{
fs.rmdir(
path.join(tmpdir.path, 'noexist.txt'),
{ recursive: true },
common.mustCall((err) => {
assert.strictEqual(err.code, 'ENOENT');
})
);
}
{
assert.rejects(
() => fs.promises.rmdir(path.join(tmpdir.path, 'noexist.txt'),
{ recursive: true }),
{
code: 'ENOENT',
}
).then(common.mustCall());
}

View File

@ -0,0 +1,29 @@
'use strict';
const common = require('../common');
const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
tmpdir.refresh();
const code = common.isWindows ? 'ENOENT' : 'ENOTDIR';
{
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
fs.writeFileSync(filePath, '');
assert.throws(() => fs.rmdirSync(filePath, { recursive: true }), { code });
}
{
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
fs.writeFileSync(filePath, '');
fs.rmdir(filePath, { recursive: true }, common.mustCall((err) => {
assert.strictEqual(err.code, code);
}));
}
{
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
fs.writeFileSync(filePath, '');
assert.rejects(() => fs.promises.rmdir(filePath, { recursive: true }),
{ code }).then(common.mustCall());
}

View File

@ -1,13 +1,13 @@
'use strict'; 'use strict';
const common = require('../common'); const common = require('../common');
const tmpdir = require('../common/tmpdir'); const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
tmpdir.refresh(); tmpdir.refresh();
{ {
// Should warn when trying to delete a file
common.expectWarning( common.expectWarning(
'DeprecationWarning', 'DeprecationWarning',
'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' + 'In future versions of Node.js, fs.rmdir(path, { recursive: true }) ' +
@ -16,5 +16,7 @@ tmpdir.refresh();
); );
const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt'); const filePath = path.join(tmpdir.path, 'rmdir-recursive.txt');
fs.writeFileSync(filePath, ''); fs.writeFileSync(filePath, '');
fs.rmdir(filePath, { recursive: true }, common.mustSucceed()); fs.rmdir(filePath, { recursive: true }, common.mustCall((err) => {
assert.strictEqual(err.code, common.isWindows ? 'ENOENT' : 'ENOTDIR');
}));
} }

View File

@ -80,8 +80,9 @@ function removeAsync(dir) {
// Recursive removal should succeed. // Recursive removal should succeed.
fs.rmdir(dir, { recursive: true }, common.mustSucceed(() => { fs.rmdir(dir, { recursive: true }, common.mustSucceed(() => {
// No error should occur if recursive and the directory does not exist. // An error should occur if recursive and the directory does not exist.
fs.rmdir(dir, { recursive: true }, common.mustSucceed(() => { fs.rmdir(dir, { recursive: true }, common.mustCall((err) => {
assert.strictEqual(err.code, 'ENOENT');
// Attempted removal should fail now because the directory is gone. // Attempted removal should fail now because the directory is gone.
fs.rmdir(dir, common.mustCall((err) => { fs.rmdir(dir, common.mustCall((err) => {
assert.strictEqual(err.syscall, 'rmdir'); assert.strictEqual(err.syscall, 'rmdir');
@ -126,8 +127,9 @@ function removeAsync(dir) {
// Recursive removal should succeed. // Recursive removal should succeed.
fs.rmdirSync(dir, { recursive: true }); fs.rmdirSync(dir, { recursive: true });
// No error should occur if recursive and the directory does not exist. // An error should occur if recursive and the directory does not exist.
fs.rmdirSync(dir, { recursive: true }); assert.throws(() => fs.rmdirSync(dir, { recursive: true }),
{ code: 'ENOENT' });
// Attempted removal should fail now because the directory is gone. // Attempted removal should fail now because the directory is gone.
assert.throws(() => fs.rmdirSync(dir), { syscall: 'rmdir' }); assert.throws(() => fs.rmdirSync(dir), { syscall: 'rmdir' });
@ -147,8 +149,9 @@ function removeAsync(dir) {
// Recursive removal should succeed. // Recursive removal should succeed.
await fs.promises.rmdir(dir, { recursive: true }); await fs.promises.rmdir(dir, { recursive: true });
// No error should occur if recursive and the directory does not exist. // An error should occur if recursive and the directory does not exist.
await fs.promises.rmdir(dir, { recursive: true }); await assert.rejects(fs.promises.rmdir(dir, { recursive: true }),
{ code: 'ENOENT' });
// Attempted removal should fail now because the directory is gone. // Attempted removal should fail now because the directory is gone.
assert.rejects(fs.promises.rmdir(dir), { syscall: 'rmdir' }); assert.rejects(fs.promises.rmdir(dir), { syscall: 'rmdir' });