Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove*() and empty*(): remove contents as Buffers to handle non-UTF-8 filenames on Linux #612

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/empty/__tests__/empty-dir-sync.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
16 changes: 16 additions & 0 deletions lib/empty/__tests__/empty-dir.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
15 changes: 11 additions & 4 deletions lib/empty/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)
})
}
Expand Down
26 changes: 26 additions & 0 deletions lib/remove/__tests__/remove-sync-dir.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
})
})
11 changes: 11 additions & 0 deletions lib/remove/__tests__/remove-sync-file.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
})
})
51 changes: 51 additions & 0 deletions lib/remove/__tests__/remove.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
18 changes: 13 additions & 5 deletions lib/remove/rimraf.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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')

Expand Down Expand Up @@ -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.
Expand Down