diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index 84f32dc6bb99db..e886973f799099 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -1,9 +1,4 @@ -// This file is a modified version of the rimraf module on npm. It has been -// modified in the following ways: -// - Use of the assert module has been replaced with core's error system. -// - All code related to the glob dependency has been removed. -// - Bring your own custom fs module is not currently supported. -// - Some basic code cleanup. +// This file is a modified version of the rimraf module on npm. 'use strict'; const { @@ -15,16 +10,12 @@ const { const { Buffer } = require('buffer'); const fs = require('fs'); const { - chmod, - chmodSync, lstat, lstatSync, readdir, readdirSync, rmdir, rmdirSync, - stat, - statSync, unlink, unlinkSync } = fs; @@ -34,9 +25,6 @@ const { sleep } = require('internal/util'); const notEmptyErrorCodes = new SafeSet(['ENOTEMPTY', 'EEXIST', 'EPERM']); const retryErrorCodes = new SafeSet( ['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']); -const isWindows = process.platform === 'win32'; -const epermHandler = isWindows ? fixWinEPERM : _rmdir; -const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync; const readdirEncoding = 'buffer'; const separator = Buffer.from(sep); @@ -69,10 +57,6 @@ function _rimraf(path, options, callback) { if (err) { if (err.code === 'ENOENT') return callback(null); - - // Windows can EPERM on stat. - if (isWindows && err.code === 'EPERM') - return fixWinEPERM(path, options, err, callback); } else if (stats.isDirectory()) { return _rmdir(path, options, err, callback); } @@ -81,11 +65,8 @@ function _rimraf(path, options, callback) { if (err) { if (err.code === 'ENOENT') return callback(null); - if (err.code === 'EISDIR') + if (err.code === 'EISDIR' || err.code === 'EPERM') return _rmdir(path, options, err, callback); - if (err.code === 'EPERM') { - return epermHandler(path, options, err, callback); - } } return callback(err); @@ -94,24 +75,6 @@ function _rimraf(path, options, callback) { } -function fixWinEPERM(path, options, originalErr, callback) { - chmod(path, 0o666, (err) => { - if (err) - return callback(err.code === 'ENOENT' ? null : originalErr); - - stat(path, (err, stats) => { - if (err) - return callback(err.code === 'ENOENT' ? null : originalErr); - - if (stats.isDirectory()) - _rmdir(path, options, originalErr, callback); - else - unlink(path, callback); - }); - }); -} - - function _rmdir(path, options, originalErr, callback) { rmdir(path, (err) => { if (err) { @@ -181,10 +144,6 @@ function rimrafSync(path, options) { } catch (err) { if (err.code === 'ENOENT') return; - - // Windows can EPERM on stat. - if (isWindows && err.code === 'EPERM') - fixWinEPERMSync(path, options, err); } try { @@ -194,14 +153,11 @@ function rimrafSync(path, options) { else _unlinkSync(path, options); } catch (err) { - if (err.code === 'ENOENT') - return; - if (err.code === 'EPERM') - return epermHandlerSync(path, options, err); - if (err.code !== 'EISDIR') - throw err; + if (err.code === 'EISDIR' || err.code === 'EPERM') + return _rmdirSync(path, options, err); - _rmdirSync(path, options, err); + if (err.code !== 'ENOENT') + throw err; } } @@ -280,31 +236,4 @@ function _rmdirSync(path, options, originalErr) { } -function fixWinEPERMSync(path, options, originalErr) { - try { - chmodSync(path, 0o666); - } catch (err) { - if (err.code === 'ENOENT') - return; - - throw originalErr; - } - - let stats; - - try { - stats = statSync(path, { throwIfNoEntry: false }); - } catch { - throw originalErr; - } - - if (stats === undefined) return; - - if (stats.isDirectory()) - _rmdirSync(path, options, originalErr); - else - _unlinkSync(path, options); -} - - module.exports = { rimraf, rimrafPromises, rimrafSync }; diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index d911feb460a2f3..3fba1e54ba60b0 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -6,7 +6,29 @@ const path = require('path'); const { isMainThread } = require('worker_threads'); function rmSync(pathname) { - fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true }); + let err = null; + const maxRetries = 10; + + for (let retryNumber = 0; retryNumber < maxRetries; ++retryNumber) { + try { + fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true }); + return; + } catch (_err) { + err = _err; + } + + const errPath = err.path; + const errCode = err.code; + + if (errCode === 'EACCES' || errCode === 'EPERM') { + const surroundingDir = path.join(errPath, '..'); + + try { fs.chmodSync(surroundingDir, 0o777); } catch {} + try { fs.chmodSync(errPath, 0o777); } catch {} + } + } + + throw err; } const testRoot = process.env.NODE_TEST_DIR ? @@ -38,7 +60,7 @@ function onexit() { try { rmSync(tmpPath); - } catch (e) { + } catch (err) { console.error('Can\'t clean tmpdir:', tmpPath); const files = fs.readdirSync(tmpPath); @@ -51,8 +73,15 @@ function onexit() { console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); } - console.error(); - throw e; + // Manually logging err instead of throwing it, so that it doesn't get + // overshadowed by an error from a test failure. + console.error(err); + + // Setting the process exit code to a non-zero exit code, so that this gets + // marked as `not ok` during a CI run. + if (!process.exitCode) { + process.exitCode = 1; + } } } diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 3fdfc8426248ac..d930832ea780fc 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -5,7 +5,6 @@ const tmpdir = require('../common/tmpdir'); const assert = require('assert'); const fs = require('fs'); const path = require('path'); -const { execSync } = require('child_process'); const { validateRmOptionsSync } = require('internal/fs/utils'); @@ -287,74 +286,134 @@ function removeAsync(dir) { // IBMi has a different access permission mechanism // This test should not be run as `root` if (!common.isIBMi && (common.isWindows || process.getuid() !== 0)) { - function makeDirectoryReadOnly(dir, mode) { - let accessErrorCode = 'EACCES'; - if (common.isWindows) { - accessErrorCode = 'EPERM'; - execSync(`icacls ${dir} /deny "everyone:(OI)(CI)(DE,DC)"`); - } else { - fs.chmodSync(dir, mode); + class InvalidStateError extends Error { + constructor(err) { + super('Invalid state'); + this.cause = err; } - return accessErrorCode; } - function makeDirectoryWritable(dir) { - if (fs.existsSync(dir)) { - if (common.isWindows) { - execSync(`icacls ${dir} /remove:d "everyone"`); - } else { - fs.chmodSync(dir, 0o777); + // Check that deleting a file that cannot be accessed using rmsync throws: + // https://github.com/nodejs/node/issues/38683 + { + function validateState(exists, err) { + // On Windows, we are allowed to access and modify the contents of a + // read-only folder. + const isValidState = + common.isWindows ? + (exists === false && err === null) : + (exists === true && err?.code === 'EACCES'); + + if (!isValidState) { + throw new InvalidStateError(err); } } - } - { - // Check that deleting a file that cannot be accessed using rmsync throws - // https://github.com/nodejs/node/issues/38683 - const dirname = nextDirPath(); - const filePath = path.join(dirname, 'text.txt'); - try { + { + const dirname = nextDirPath(); + const filePath = path.join(dirname, 'text.txt'); + fs.mkdirSync(dirname, { recursive: true }); fs.writeFileSync(filePath, 'hello'); - const code = makeDirectoryReadOnly(dirname, 0o444); - assert.throws(() => { + + fs.chmodSync(filePath, 0o444); + fs.chmodSync(dirname, 0o444); + + let err = null; + + try { fs.rmSync(filePath, { force: true }); - }, { - code, - name: 'Error', - }); - } finally { - makeDirectoryWritable(dirname); + } catch (_err) { + err = _err; + } + + try { fs.chmodSync(dirname, 0o777); } catch {} + try { fs.chmodSync(filePath, 0o777); } catch {} + + validateState(fs.existsSync(filePath), err); + } + + { + const dirname = nextDirPath(); + const filePath = path.join(dirname, 'text.txt'); + + fs.mkdirSync(dirname, { recursive: true }); + fs.writeFileSync(filePath, 'hello'); + + fs.chmodSync(filePath, 0o444); + fs.chmodSync(dirname, 0o444); + + fs.rm(filePath, { force: true }, common.mustCall((err) => { + try { fs.chmodSync(dirname, 0o777); } catch {} + try { fs.chmodSync(filePath, 0o777); } catch {} + + validateState(fs.existsSync(filePath), err); + })); } } + // Check endless recursion. + // https://github.com/nodejs/node/issues/34580 { - // Check endless recursion. - // https://github.com/nodejs/node/issues/34580 - const dirname = nextDirPath(); - fs.mkdirSync(dirname, { recursive: true }); - const root = fs.mkdtempSync(path.join(dirname, 'fs-')); - const middle = path.join(root, 'middle'); - fs.mkdirSync(middle); - fs.mkdirSync(path.join(middle, 'leaf')); // Make `middle` non-empty - try { - const code = makeDirectoryReadOnly(middle, 0o555); + function validateState(exists, err) { + // On Windows, we are not allowed to delete a read-only folder yet. + // TODO(RaisinTen): Remove Windows special-casing if this lands: + // https://github.com/libuv/libuv/pull/3193 + const isValidState = + exists === true && + err?.code === common.isWindows ? 'EPERM' : 'EACCES'; + + if (!isValidState) { + throw new InvalidStateError(err); + } + } + + { + const dirname = nextDirPath(); + fs.mkdirSync(dirname, { recursive: true }); + const root = fs.mkdtempSync(path.join(dirname, 'fs-')); + const middle = path.join(root, 'middle'); + const leaf = path.join(middle, 'leaf'); + + fs.mkdirSync(middle); + fs.mkdirSync(leaf); + + fs.chmodSync(leaf, 0o555); + fs.chmodSync(middle, 0o555); + + let err = null; + try { - assert.throws(() => { - fs.rmSync(root, { recursive: true }); - }, { - code, - name: 'Error', - }); - } catch (err) { - // Only fail the test if the folder was not deleted. - // as in some cases rmSync succesfully deletes read-only folders. - if (fs.existsSync(root)) { - throw err; - } + fs.rmSync(root, { recursive: true }); + } catch (_err) { + err = _err; } - } finally { - makeDirectoryWritable(middle); + + try { fs.chmodSync(middle, 0o777); } catch {} + try { fs.chmodSync(leaf, 0o777); } catch {} + + validateState(fs.existsSync(root), err); + } + + { + const dirname = nextDirPath(); + fs.mkdirSync(dirname, { recursive: true }); + const root = fs.mkdtempSync(path.join(dirname, 'fs-')); + const middle = path.join(root, 'middle'); + const leaf = path.join(middle, 'leaf'); + + fs.mkdirSync(middle); + fs.mkdirSync(leaf); + + fs.chmodSync(leaf, 0o555); + fs.chmodSync(middle, 0o555); + + fs.rm(root, { recursive: true }, common.mustCall((err) => { + try { fs.chmodSync(middle, 0o777); } catch {} + try { fs.chmodSync(leaf, 0o777); } catch {} + + validateState(fs.existsSync(root), err); + })); } } }