Skip to content

Commit

Permalink
refactor: replace callback-style API with Promise-style API (#90)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: drops callback-style support in favor of a Promises-based API. If you still need callback-style support, see [`util.callbackify`](https://nodejs.org/dist/latest-v12.x/docs/api/util.html#util_util_callbackify_original).

Additional changes:

* Refactor the code so that's it's encapsulated in a class, instead of in one monolithic function (with nested functions)
* More modern Node refactors
* Replace `tape` with `ava`
* Replace `concat-stream` with `get-stream`
* Replace `rimraf` (& `fs.existsSync` usage) with `fs-extra`
* The code base should have full test coverage now
  • Loading branch information
@malept
malept authored Mar 25, 2020
1 parent 7993cb8 commit af00186
Show file tree
Hide file tree
Showing 6 changed files with 318 additions and 400 deletions.
310 changes: 135 additions & 175 deletions index.js
View file
Original file line number Diff line number Diff line change
@@ -1,215 +1,175 @@
const fs = require('fs')
const debug = require('debug')('extract-zip')
const { createWriteStream, promises: fs } = require('fs')
const getStream = require('get-stream')
const path = require('path')
const { promisify } = require('util')
const stream = require('stream')
const yauzl = require('yauzl')
const concat = require('concat-stream')
const debug = require('debug')('extract-zip')

module.exports = function (zipPath, opts, cb) {
debug('creating target directory', opts.dir)
const openZip = promisify(yauzl.open)
const pipeline = promisify(stream.pipeline)

if (path.isAbsolute(opts.dir) === false) {
return cb(new Error('Target directory is expected to be absolute'))
class Extractor {
constructor (zipPath, opts) {
this.zipPath = zipPath
this.opts = opts
}

fs.mkdir(opts.dir, { recursive: true }, function (err) {
if (err) return cb(err)

fs.realpath(opts.dir, function (err, canonicalDir) {
if (err) return cb(err)

opts.dir = canonicalDir

openZip(opts)
})
})

function openZip () {
debug('opening', zipPath, 'with opts', opts)
async extract () {
debug('opening', this.zipPath, 'with opts', this.opts)

yauzl.open(zipPath, { lazyEntries: true }, function (err, zipfile) {
if (err) return cb(err)
this.zipfile = await openZip(this.zipPath, { lazyEntries: true })
this.canceled = false

let cancelled = false

zipfile.on('error', function (err) {
if (err) {
cancelled = true
return cb(err)
}
return new Promise((resolve, reject) => {
this.zipfile.on('error', err => {
this.canceled = true
reject(err)
})
zipfile.readEntry()
this.zipfile.readEntry()

zipfile.on('close', function () {
if (!cancelled) {
this.zipfile.on('close', () => {
if (!this.canceled) {
debug('zip extraction complete')
cb()
resolve()
}
})

zipfile.on('entry', function (entry) {
this.zipfile.on('entry', async entry => {
/* istanbul ignore if */
if (cancelled) {
debug('skipping entry', entry.fileName, { cancelled: cancelled })
if (this.canceled) {
debug('skipping entry', entry.fileName, { cancelled: this.canceled })
return
}

debug('zipfile entry', entry.fileName)

if (entry.fileName.startsWith('__MACOSX/')) {
zipfile.readEntry()
this.zipfile.readEntry()
return
}

const destDir = path.dirname(path.join(opts.dir, entry.fileName))
const destDir = path.dirname(path.join(this.opts.dir, entry.fileName))

fs.mkdir(destDir, { recursive: true }, function (err) {
/* istanbul ignore if */
if (err) {
cancelled = true
zipfile.close()
return cb(err)
}
try {
await fs.mkdir(destDir, { recursive: true })

fs.realpath(destDir, function (err, canonicalDestDir) {
/* istanbul ignore if */
if (err) {
cancelled = true
zipfile.close()
return cb(err)
}

const relativeDestDir = path.relative(opts.dir, canonicalDestDir)

if (relativeDestDir.split(path.sep).indexOf('..') !== -1) {
cancelled = true
zipfile.close()
return cb(new Error('Out of bound path "' + canonicalDestDir + '" found while processing file ' + entry.fileName))
}

extractEntry(entry, function (err) {
// if any extraction fails then abort everything
if (err) {
cancelled = true
zipfile.close()
return cb(err)
}
debug('finished processing', entry.fileName)
zipfile.readEntry()
})
})
})
})
const canonicalDestDir = await fs.realpath(destDir)
const relativeDestDir = path.relative(this.opts.dir, canonicalDestDir)

function extractEntry (entry, done) {
/* istanbul ignore if */
if (cancelled) {
debug('skipping entry extraction', entry.fileName, { cancelled: cancelled })
return setImmediate(done)
}
if (relativeDestDir.split(path.sep).includes('..')) {
throw new Error(`Out of bound path "${canonicalDestDir}" found while processing file ${entry.fileName}`)
}

if (opts.onEntry) {
opts.onEntry(entry, zipfile)
await this.extractEntry(entry)
debug('finished processing', entry.fileName)
this.zipfile.readEntry()
} catch (err) {
this.canceled = true
this.zipfile.close()
reject(err)
}
})
})
}

const dest = path.join(opts.dir, entry.fileName)

// convert external file attr int into a fs stat mode int
let mode = (entry.externalFileAttributes >> 16) & 0xFFFF
// check if it's a symlink or dir (using stat mode constants)
const IFMT = 61440
const IFDIR = 16384
const IFLNK = 40960
const symlink = (mode & IFMT) === IFLNK
let isDir = (mode & IFMT) === IFDIR
async extractEntry (entry) {
/* istanbul ignore if */
if (this.canceled) {
debug('skipping entry extraction', entry.fileName, { cancelled: this.canceled })
return
}

if (this.opts.onEntry) {
this.opts.onEntry(entry, this.zipfile)
}

const dest = path.join(this.opts.dir, entry.fileName)

// convert external file attr int into a fs stat mode int
const mode = (entry.externalFileAttributes >> 16) & 0xFFFF
// check if it's a symlink or dir (using stat mode constants)
const IFMT = 61440
const IFDIR = 16384
const IFLNK = 40960
const symlink = (mode & IFMT) === IFLNK
let isDir = (mode & IFMT) === IFDIR

// Failsafe, borrowed from jsZip
if (!isDir && entry.fileName.endsWith('/')) {
isDir = true
}

// check for windows weird way of specifying a directory
// /~https://github.com/maxogden/extract-zip/issues/13#issuecomment-154494566
const madeBy = entry.versionMadeBy >> 8
if (!isDir) isDir = (madeBy === 0 && entry.externalFileAttributes === 16)

debug('extracting entry', { filename: entry.fileName, isDir: isDir, isSymlink: symlink })

// reverse umask first (~)
const umask = ~process.umask()
// & with processes umask to override invalid perms
const procMode = this.getExtractedMode(mode, isDir) & umask

// always ensure folders are created
const destDir = isDir ? dest : path.dirname(dest)

const mkdirOptions = { recursive: true }
if (isDir) {
mkdirOptions.mode = procMode
}
debug('mkdir', { dir: destDir, ...mkdirOptions })
await fs.mkdir(destDir, mkdirOptions)
if (isDir) return

debug('opening read stream', dest)
const readStream = await promisify(this.zipfile.openReadStream.bind(this.zipfile))(entry)

if (symlink) {
const link = await getStream(readStream)
debug('creating symlink', link, dest)
await fs.symlink(link, dest)
} else {
await pipeline(readStream, createWriteStream(dest, { mode: procMode }))
}
}

// Failsafe, borrowed from jsZip
if (!isDir && entry.fileName.slice(-1) === '/') {
isDir = true
getExtractedMode (entryMode, isDir) {
let mode = entryMode
// Set defaults, if necessary
if (mode === 0) {
if (isDir) {
if (this.opts.defaultDirMode) {
mode = parseInt(this.opts.defaultDirMode, 10)
}

// check for windows weird way of specifying a directory
// /~https://github.com/maxogden/extract-zip/issues/13#issuecomment-154494566
const madeBy = entry.versionMadeBy >> 8
if (!isDir) isDir = (madeBy === 0 && entry.externalFileAttributes === 16)

// if no mode then default to default modes
if (mode === 0) {
if (isDir) {
if (opts.defaultDirMode) mode = parseInt(opts.defaultDirMode, 10)
if (!mode) mode = 0o755
} else {
if (opts.defaultFileMode) mode = parseInt(opts.defaultFileMode, 10)
if (!mode) mode = 0o644
}
if (!mode) {
mode = 0o755
}
} else {
if (this.opts.defaultFileMode) {
mode = parseInt(this.opts.defaultFileMode, 10)
}

debug('extracting entry', { filename: entry.fileName, isDir: isDir, isSymlink: symlink })

// reverse umask first (~)
const umask = ~process.umask()
// & with processes umask to override invalid perms
const procMode = mode & umask
if (!mode) {
mode = 0o644
}
}
}

// always ensure folders are created
const destDir = isDir ? dest : path.dirname(dest)
return mode
}
}

debug('mkdirp', { dir: destDir })
fs.mkdir(destDir, { recursive: true }, function (err) {
/* istanbul ignore if */
if (err) {
debug('mkdirp error', destDir, { error: err })
cancelled = true
return done(err)
}
module.exports = async function (zipPath, opts) {
debug('creating target directory', opts.dir)

if (isDir) return done()

debug('opening read stream', dest)
zipfile.openReadStream(entry, function (err, readStream) {
/* istanbul ignore if */
if (err) {
debug('openReadStream error', err)
cancelled = true
return done(err)
}

readStream.on('error', function (err) {
/* istanbul ignore next */
console.log('read err', err)
})

if (symlink) writeSymlink()
else writeStream()

function writeStream () {
const writeStream = fs.createWriteStream(dest, { mode: procMode })
readStream.pipe(writeStream)

writeStream.on('finish', function () {
done()
})

writeStream.on('error', /* istanbul ignore next */ function (err) {
debug('write error', { error: err })
cancelled = true
return done(err)
})
}

// AFAICT the content of the symlink file itself is the symlink target filename string
function writeSymlink () {
readStream.pipe(concat(function (data) {
const link = data.toString()
debug('creating symlink', link, dest)
fs.symlink(link, dest, function (err) {
if (err) cancelled = true
done(err)
})
}))
}
})
})
}
})
if (!path.isAbsolute(opts.dir)) {
throw new Error('Target directory is expected to be absolute')
}

await fs.mkdir(opts.dir, { recursive: true })
opts.dir = await fs.realpath(opts.dir)
return new Extractor(zipPath, opts).extract()
}
13 changes: 7 additions & 6 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
"extract-zip": "cli.js"
},
"scripts": {
"coverage": "nyc node test/test.js",
"ava": "ava",
"coverage": "nyc ava",
"lint": "standard",
"test": "node test/test.js"
"test": "npm run lint && ava"
},
"files": [
"*.js"
Expand All @@ -26,15 +27,15 @@
"node": ">= 10.12.0"
},
"dependencies": {
"concat-stream": "^2.0.0",
"debug": "^4.1.1",
"get-stream": "^5.1.0",
"yauzl": "^2.10.0"
},
"devDependencies": {
"ava": "^3.5.1",
"fs-extra": "^9.0.0",
"nyc": "^15.0.0",
"rimraf": "^3.0.2",
"standard": "^14.3.3",
"tape": "^4.2.0"
"standard": "^14.3.3"
},
"directories": {
"test": "test"
Expand Down
Loading

0 comments on commit af00186

Please sign in to comment.