From 4b228c9197ed84f38cfe65c3c89245619441cd74 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 24 May 2021 01:01:07 -0700 Subject: [PATCH] do not ever actually try to rmdir / When preserveRoot is set to false, we may remove all the children of the root directory, but we'll never actually succeed in rmdir()-ing the root dir itself, so don't try. --- lib/rimraf-posix.js | 12 +++++++++++- lib/rimraf-windows.js | 15 ++++++++++++--- test/rimraf-posix.js | 29 +++++++++++++++++++++++++++++ test/rimraf-windows.js | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/lib/rimraf-posix.js b/lib/rimraf-posix.js index 70694d93..fe2fd007 100644 --- a/lib/rimraf-posix.js +++ b/lib/rimraf-posix.js @@ -14,7 +14,7 @@ const { }, } = require('./fs.js') -const { resolve } = require('path') +const { resolve, parse } = require('path') const { readdirOrError, @@ -38,6 +38,12 @@ const rimrafPosix = async (path, opt) => { await Promise.all(entries.map(entry => rimrafPosix(resolve(path, entry), opt))) + // we don't ever ACTUALLY try to unlink /, because that can never work + // but when preserveRoot is false, we could be operating on it. + // No need to check if preserveRoot is not false. + if (opt.preserveRoot === false && path === parse(path).root) + return + return ignoreENOENT(rmdir(path)) } @@ -52,6 +58,10 @@ const rimrafPosixSync = (path, opt) => { } for (const entry of entries) rimrafPosixSync(resolve(path, entry), opt) + + if (opt.preserveRoot === false && path === parse(path).root) + return + return ignoreENOENTSync(() => rmdirSync(path)) } diff --git a/lib/rimraf-windows.js b/lib/rimraf-windows.js index 3858c436..bc49267b 100644 --- a/lib/rimraf-windows.js +++ b/lib/rimraf-windows.js @@ -9,7 +9,7 @@ // This works around the fact that unlink/rmdir is non-atomic and takes // a non-deterministic amount of time to complete. -const { resolve, basename } = require('path') +const { resolve, basename, parse } = require('path') const { defaultTmp, defaultTmpSync } = require('./default-tmp.js') const { @@ -75,7 +75,7 @@ const rimrafWindows = async (path, opt) => { if (!opt.tmp) return rimrafWindows(path, { ...opt, tmp: await defaultTmp(path) }) - if (path === opt.tmp) + if (path === opt.tmp && parse(path).root !== path) throw new Error('cannot delete temp directory used for deletion') const entries = await readdirOrError(path) @@ -92,6 +92,12 @@ const rimrafWindows = async (path, opt) => { await Promise.all(entries.map(entry => rimrafWindows(resolve(path, entry), opt))) + // we don't ever ACTUALLY try to unlink /, because that can never work + // but when preserveRoot is false, we could be operating on it. + // No need to check if preserveRoot is not false. + if (opt.preserveRoot === false && path === parse(path).root) + return + return await ignoreENOENT(tmpUnlink(path, opt.tmp, rmdir)) } @@ -105,7 +111,7 @@ const rimrafWindowsSync = (path, opt) => { if (!opt.tmp) return rimrafWindowsSync(path, { ...opt, tmp: defaultTmpSync(path) }) - if (path === opt.tmp) + if (path === opt.tmp && parse(path).root !== path) throw new Error('cannot delete temp directory used for deletion') const entries = readdirOrErrorSync(path) @@ -123,6 +129,9 @@ const rimrafWindowsSync = (path, opt) => { for (const entry of entries) rimrafWindowsSync(resolve(path, entry), opt) + if (opt.preserveRoot === false && path === parse(path).root) + return + return ignoreENOENTSync(() => tmpUnlinkSync(path, opt.tmp, rmdirSync)) } diff --git a/test/rimraf-posix.js b/test/rimraf-posix.js index 4e73d2fa..a34ef38d 100644 --- a/test/rimraf-posix.js +++ b/test/rimraf-posix.js @@ -170,3 +170,32 @@ t.test('ignore ENOENTs from unlink/rmdir', async t => { t.end() }) + +t.test('rimraffing root, do not actually rmdir root', async t => { + let ROOT = null + const { parse } = require('path') + const { rimrafPosix, rimrafPosixSync } = t.mock('../lib/rimraf-posix.js', { + path: { + ...require('path'), + parse: (path) => { + const p = parse(path) + if (path === ROOT) + p.root = path + return p + }, + }, + }) + t.test('async', async t => { + ROOT = t.testdir(fixture) + await rimrafPosix(ROOT, { preserveRoot: false }) + t.equal(fs.statSync(ROOT).isDirectory(), true, 'root still present') + t.same(fs.readdirSync(ROOT), [], 'entries all gone') + }) + t.test('sync', async t => { + ROOT = t.testdir(fixture) + rimrafPosixSync(ROOT, { preserveRoot: false }) + t.equal(fs.statSync(ROOT).isDirectory(), true, 'root still present') + t.same(fs.readdirSync(ROOT), [], 'entries all gone') + }) + t.end() +}) diff --git a/test/rimraf-windows.js b/test/rimraf-windows.js index fc6ea79e..5639d1a8 100644 --- a/test/rimraf-windows.js +++ b/test/rimraf-windows.js @@ -449,3 +449,36 @@ t.test('handle EPERMs, chmod raises something other than ENOENT', async t => { }) t.end() }) + +t.test('rimraffing root, do not actually rmdir root', async t => { + const fs = require('../lib/fs.js') + let ROOT = null + const { parse } = require('path') + const { + rimrafWindows, + rimrafWindowsSync, + } = t.mock('../lib/rimraf-windows.js', { + path: { + ...require('path'), + parse: (path) => { + const p = parse(path) + if (path === ROOT) + p.root = path + return p + }, + }, + }) + t.test('async', async t => { + ROOT = t.testdir(fixture) + await rimrafWindows(ROOT, { preserveRoot: false }) + t.equal(fs.statSync(ROOT).isDirectory(), true, 'root still present') + t.same(fs.readdirSync(ROOT), [], 'entries all gone') + }) + t.test('sync', async t => { + ROOT = t.testdir(fixture) + rimrafWindowsSync(ROOT, { preserveRoot: false }) + t.equal(fs.statSync(ROOT).isDirectory(), true, 'root still present') + t.same(fs.readdirSync(ROOT), [], 'entries all gone') + }) + t.end() +})