remove*() and empty*(): list and remove contents as Buffers to handle…

… non-UTF-8 filenames on Linux
jprichardson · Aug 5, 2018 · 2ddf1d5 · 2ddf1d5
1 parent fb3dda7
commit 2ddf1d5
Show file tree

Hide file tree

Showing 7 changed files with 141 additions and 9 deletions.
diff --git a/lib/empty/__tests__/empty-dir-sync.test.js b/lib/empty/__tests__/empty-dir-sync.test.js
@@ -33,6 +33,19 @@ describe('+ emptyDir()', () => {
       fse.emptyDirSync(TEST_DIR)
       assert.strictEqual(fs.readdirSync(TEST_DIR).length, 0)
     })
+
+    it('should delete contained items with invalid UTF-8 names', () => {
+      // verify nothing
+      assert.strictEqual(fs.readdirSync(TEST_DIR).length, 0)
+      const base = Buffer.from(path.join(TEST_DIR, 'some-'), 'utf8')
+      fs.writeFileSync(Buffer.concat([base, Buffer.from([0x80])]), '')
+      fs.writeFileSync(Buffer.concat([base, Buffer.from([0x41, 0xDF])]), '')
+      fs.mkdirSync(Buffer.concat([base, Buffer.from([0x41, 0x41, 0xFF])]))
+      assert.strictEqual(fs.readdirSync(TEST_DIR).length, 3)
+
+      fse.emptyDirSync(TEST_DIR)
+      assert.strictEqual(fs.readdirSync(TEST_DIR).length, 0)
+    })
   })
 
   describe('> when directory exists and contains no items', () => {

diff --git a/lib/empty/__tests__/empty-dir.test.js b/lib/empty/__tests__/empty-dir.test.js
@@ -36,6 +36,22 @@ describe('+ emptyDir()', () => {
         done()
       })
     })
+
+    it('should delete contained items with invalid UTF-8 names', done => {
+      // verify nothing
+      assert.strictEqual(fs.readdirSync(TEST_DIR).length, 0)
+      const base = Buffer.from(path.join(TEST_DIR, 'some-'), 'utf8')
+      fs.writeFileSync(Buffer.concat([base, Buffer.from([0x80])]), '')
+      fs.writeFileSync(Buffer.concat([base, Buffer.from([0x41, 0xDF])]), '')
+      fs.mkdirSync(Buffer.concat([base, Buffer.from([0x41, 0x41, 0xFF])]))
+      assert.strictEqual(fs.readdirSync(TEST_DIR).length, 3)
+
+      fse.emptyDir(TEST_DIR, err => {
+        assert.ifError(err)
+        assert.strictEqual(fs.readdirSync(TEST_DIR).length, 0)
+        done()
+      })
+    })
   })
 
   describe('> when directory exists and contains no items', () => {

diff --git a/lib/empty/index.js b/lib/empty/index.js
@@ -6,12 +6,17 @@ const path = require('path')
 const mkdir = require('../mkdirs')
 const remove = require('../remove')
 
+const seperator = Buffer.from(path.sep, 'utf8')
+
 const emptyDir = u(function emptyDir (dir, callback) {
   callback = callback || function () {}
-  fs.readdir(dir, (err, items) => {
+
+  const dirBuf = Buffer.isBuffer(dir) ? dir : Buffer.from(dir, 'utf8')
+
+  fs.readdir(dir, {encoding: 'buffer'}, (err, items) => {
     if (err) return mkdir.mkdirs(dir, callback)
 
-    items = items.map(item => path.join(dir, item))
+    items = items.map(item => Buffer.concat([dirBuf, seperator, item]))
 
     deleteItem()
 
@@ -27,15 +32,17 @@ const emptyDir = u(function emptyDir (dir, callback) {
 })
 
 function emptyDirSync (dir) {
+  const dirBuf = Buffer.isBuffer(dir) ? dir : Buffer.from(dir, 'utf8')
+
   let items
   try {
-    items = fs.readdirSync(dir)
+    items = fs.readdirSync(dir, {encoding: 'buffer'})
   } catch (err) {
     return mkdir.mkdirsSync(dir)
   }
 
   items.forEach(item => {
-    item = path.join(dir, item)
+    item = Buffer.concat([dirBuf, seperator, item])
     remove.removeSync(item)
   })
 }

diff --git a/lib/remove/__tests__/remove-sync-dir.test.js b/lib/remove/__tests__/remove-sync-dir.test.js
@@ -29,5 +29,31 @@ describe('remove/sync', () => {
       fse.removeSync(TEST_DIR)
       assert(!fs.existsSync(TEST_DIR))
     })
+
+    it('should delete contained files and dirs with invalid UTF-8 names', () => {
+      assert(fs.existsSync(TEST_DIR))
+      const separator = Buffer.from(path.sep, 'utf8')
+      const base = Buffer.from(TEST_DIR, 'utf8')
+      const subdir = Buffer.concat([base, separator, Buffer.from([0x80])])
+      const file = Buffer.concat([subdir, separator, Buffer.from([0x41, 0xDF])])
+      fs.mkdirSync(subdir)
+      assert(fs.existsSync(subdir))
+      fs.writeFileSync(file, 'somedata')
+      fse.removeSync(TEST_DIR)
+      assert(!fs.existsSync(TEST_DIR))
+    })
+
+    it('should delete dir with invalid UTF-8 name passed as a Buffer', () => {
+      assert(fs.existsSync(TEST_DIR))
+      const separator = Buffer.from(path.sep, 'utf8')
+      const base = Buffer.from(TEST_DIR, 'utf8')
+      const subdir = Buffer.concat([base, separator, Buffer.from([0x80])])
+      const file = Buffer.concat([subdir, separator, Buffer.from([0x41, 0xDF])])
+      fs.mkdirSync(subdir)
+      assert(fs.existsSync(subdir))
+      fs.writeFileSync(file, 'somedata')
+      fse.removeSync(subdir)
+      assert(!fs.existsSync(subdir))
+    })
   })
 })
diff --git a/lib/remove/__tests__/remove-sync-file.test.js b/lib/remove/__tests__/remove-sync-file.test.js
@@ -24,5 +24,16 @@ describe('remove/sync', () => {
       fse.removeSync(file)
       assert(!fs.existsSync(file))
     })
+
+    it('should delete file with invalid UTF-8 name passed as a Buffer', () => {
+      assert(fs.existsSync(TEST_DIR))
+      const separator = Buffer.from(path.sep, 'utf8')
+      const base = Buffer.from(TEST_DIR, 'utf8')
+      const file = Buffer.concat([base, separator, Buffer.from([0x41, 0xDF])])
+      fs.writeFileSync(file, 'hello')
+      assert(fs.existsSync(file))
+      fse.removeSync(file)
+      assert(!fs.existsSync(file))
+    })
   })
 })
diff --git a/lib/remove/__tests__/remove.test.js b/lib/remove/__tests__/remove.test.js
@@ -65,6 +65,57 @@ describe('remove', () => {
       })
     })
 
+    it('should delete a directory full of directories and files with invalid UTF-8 names', done => {
+      const separator = Buffer.from(path.sep, 'utf8')
+      const base = Buffer.from(TEST_DIR, 'utf8')
+      const subdir = Buffer.concat([base, separator, Buffer.from([0x80])])
+      const file = Buffer.concat([subdir, separator, Buffer.from([0x41, 0xDF])])
+
+      fs.mkdirSync(subdir)
+      assert(fs.existsSync(subdir))
+      fs.writeFileSync(file, 'somedata')
+      assert(fs.readdirSync(TEST_DIR).length === 1)
+
+      fse.remove(TEST_DIR, err => {
+        assert.ifError(err)
+        assert(!fs.existsSync(TEST_DIR))
+        done()
+      })
+    })
+
+    it('should delete a directory with invalid UTF-8 name passed as a Buffer', done => {
+      const separator = Buffer.from(path.sep, 'utf8')
+      const base = Buffer.from(TEST_DIR, 'utf8')
+      const subdir = Buffer.concat([base, separator, Buffer.from([0x80])])
+      const file = Buffer.concat([subdir, separator, Buffer.from([0x41, 0xDF])])
+
+      fs.mkdirSync(subdir)
+      assert(fs.existsSync(subdir))
+      fs.writeFileSync(file, 'somedata')
+      assert(fs.readdirSync(TEST_DIR).length === 1)
+
+      fse.remove(subdir, err => {
+        assert.ifError(err)
+        assert(!fs.existsSync(subdir))
+        done()
+      })
+    })
+
+    it('should delete a file with invalid UTF-8 name passed as a Buffer', done => {
+      const separator = Buffer.from(path.sep, 'utf8')
+      const base = Buffer.from(TEST_DIR, 'utf8')
+      const file = Buffer.concat([base, separator, Buffer.from([0x41, 0xDF])])
+
+      fs.writeFileSync(file, 'somedata')
+      assert(fs.existsSync(file))
+
+      fse.remove(file, err => {
+        assert.ifError(err)
+        assert(!fs.existsSync(file))
+        done()
+      })
+    })
+
     it('should delete without a callback', done => {
       const file = path.join(TEST_DIR, 'file')
       fs.writeFileSync(file, 'hello')

diff --git a/lib/remove/rimraf.js b/lib/remove/rimraf.js
@@ -5,6 +5,7 @@ const path = require('path')
 const assert = require('assert')
 
 const isWindows = (process.platform === 'win32')
+const seperator = Buffer.from(path.sep, 'utf8')
 
 function defaults (options) {
   const methods = [
@@ -33,7 +34,7 @@ function rimraf (p, options, cb) {
   }
 
   assert(p, 'rimraf: missing path')
-  assert.strictEqual(typeof p, 'string', 'rimraf: path should be a string')
+  assert(typeof p === 'string' || Buffer.isBuffer(p), 'rimraf: path should be a string or Buffer')
   assert.strictEqual(typeof cb, 'function', 'rimraf: callback function required')
   assert(options, 'rimraf: invalid options argument provided')
   assert.strictEqual(typeof options, 'object', 'rimraf: options should be object')
@@ -197,7 +198,10 @@ function rmkids (p, options, cb) {
   assert(options)
   assert(typeof cb === 'function')
 
-  options.readdir(p, (er, files) => {
+  // Make a Buffer from p for later path concatenation
+  const pBuf = Buffer.isBuffer(p) ? p : Buffer.from(p, 'utf8')
+
+  options.readdir(p, {encoding: 'buffer'}, (er, files) => {
     if (er) return cb(er)
 
     let n = files.length
@@ -206,7 +210,7 @@ function rmkids (p, options, cb) {
     if (n === 0) return options.rmdir(p, cb)
 
     files.forEach(f => {
-      rimraf(path.join(p, f), options, er => {
+      rimraf(Buffer.concat([pBuf, seperator, f]), options, er => {
         if (errState) {
           return
         }
@@ -229,7 +233,7 @@ function rimrafSync (p, options) {
   defaults(options)
 
   assert(p, 'rimraf: missing path')
-  assert.strictEqual(typeof p, 'string', 'rimraf: path should be a string')
+  assert(typeof p === 'string' || Buffer.isBuffer(p), 'rimraf: path should be a string or Buffer')
   assert(options, 'rimraf: missing options')
   assert.strictEqual(typeof options, 'object', 'rimraf: options should be object')
 
@@ -288,7 +292,11 @@ function rmdirSync (p, options, originalEr) {
 function rmkidsSync (p, options) {
   assert(p)
   assert(options)
-  options.readdirSync(p).forEach(f => rimrafSync(path.join(p, f), options))
+
+  // Make a Buffer from p for later path concatenation
+  const pBuf = Buffer.isBuffer(p) ? p : Buffer.from(p, 'utf8')
+
+  options.readdirSync(p, {encoding: 'buffer'}).forEach(f => rimrafSync(Buffer.concat([pBuf, seperator, f]), options))
 
   // We only end up here once we got ENOTEMPTY at least once, and
   // at this point, we are guaranteed to have removed all the kids.