From 7993cb87dc8f474cd4bd1c08f75e52acb837fbf6 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 20:35:55 -0700 Subject: [PATCH] feat: require Node 10.12 (#89) BREAKING CHANGE: drop support for Node < 10.12 * build: replace Travis CI with GitHub Actions * refactor: remove use of temp module * test: don't check for the exact tmp dir * test: do not run some symlink tests on Windows * feat: drop use of mkdirp * docs: update badges * build(deps): upgrade debug to ^4.1.1 * build(deps-dev): upgrade rimraf to ^3.0.2 * build(deps-dev): upgrade standard to ^14.3.3 * refactor: remove use of deprecated fs.exists * build(deps): upgrade concat-stream to ^2.0.0 * test: add nyc for code coverage * test: add test for onEntry option * refactor: use const/let instead of var * refactor: use octal literals * refactor: use String.prototype.startsWith * test: test the symlink's link * docs: note Node version requirement in the readme * docs: fix dir description --- .github/workflows/ci.yml | 41 ++++++++++ .gitignore | 1 + .travis.yml | 9 --- cli.js | 2 +- index.js | 78 ++++++++++--------- package.json | 20 +++-- readme.md | 12 +-- test/test.js | 164 ++++++++++++++++++++------------------- 8 files changed, 188 insertions(+), 139 deletions(-) create mode 100644 .github/workflows/ci.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..09537fc --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,41 @@ +name: CI + +on: + push: + branches: + - master + tags: + - v[0-9]+.[0-9]+.[0-9]+* + pull_request: + +jobs: + build: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [windows-latest, macOS-latest, ubuntu-latest] + node-version: [10.x, 12.x] + + steps: + - name: Fix git checkout line endings + run: git config --global core.autocrlf input + - uses: actions/checkout@v2 + - name: Setup Node.js + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} + - name: Get yarn cache + id: yarn-cache + run: echo "::set-output name=dir::$(yarn cache dir)" + - uses: actions/cache@v1 + with: + path: ${{ steps.yarn-cache.outputs.dir }} + key: ${{ runner.os }}-yarn-${{ hashFiles('**/package.json') }} + restore-keys: | + ${{ runner.os }}-yarn- + - name: Install + run: yarn + - name: Lint + run: yarn lint + - name: Test + run: yarn coverage diff --git a/.gitignore b/.gitignore index 01198e3..248ed3b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .DS_Store +.nyc_output node_modules package-lock.json yarn.lock diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 2e470e0..0000000 --- a/.travis.yml +++ /dev/null @@ -1,9 +0,0 @@ -sudo: false -language: node_js -node_js: - - '0.12' - - 'iojs' - - '4' - - '6' - - '8' - - '10' diff --git a/cli.js b/cli.js index 76c337d..53f0160 100755 --- a/cli.js +++ b/cli.js @@ -10,7 +10,7 @@ if (!source) { process.exit(1) } -extract(source, {dir: dest}, function (err, results) { +extract(source, { dir: dest }, function (err, results) { if (err) { console.error('error!', err) process.exit(1) diff --git a/index.js b/index.js index 682d401..915f2ca 100644 --- a/index.js +++ b/index.js @@ -1,9 +1,8 @@ -var fs = require('fs') -var path = require('path') -var yauzl = require('yauzl') -var mkdirp = require('mkdirp') -var concat = require('concat-stream') -var debug = require('debug')('extract-zip') +const fs = require('fs') +const path = require('path') +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) @@ -12,7 +11,7 @@ module.exports = function (zipPath, opts, cb) { return cb(new Error('Target directory is expected to be absolute')) } - mkdirp(opts.dir, function (err) { + fs.mkdir(opts.dir, { recursive: true }, function (err) { if (err) return cb(err) fs.realpath(opts.dir, function (err, canonicalDir) { @@ -27,10 +26,10 @@ module.exports = function (zipPath, opts, cb) { function openZip () { debug('opening', zipPath, 'with opts', opts) - yauzl.open(zipPath, {lazyEntries: true}, function (err, zipfile) { + yauzl.open(zipPath, { lazyEntries: true }, function (err, zipfile) { if (err) return cb(err) - var cancelled = false + let cancelled = false zipfile.on('error', function (err) { if (err) { @@ -48,22 +47,23 @@ module.exports = function (zipPath, opts, cb) { }) zipfile.on('entry', function (entry) { + /* istanbul ignore if */ if (cancelled) { - debug('skipping entry', entry.fileName, {cancelled: cancelled}) + debug('skipping entry', entry.fileName, { cancelled: cancelled }) return } debug('zipfile entry', entry.fileName) - if (/^__MACOSX\//.test(entry.fileName)) { - // dir name starts with __MACOSX/ + if (entry.fileName.startsWith('__MACOSX/')) { zipfile.readEntry() return } - var destDir = path.dirname(path.join(opts.dir, entry.fileName)) + const destDir = path.dirname(path.join(opts.dir, entry.fileName)) - mkdirp(destDir, function (err) { + fs.mkdir(destDir, { recursive: true }, function (err) { + /* istanbul ignore if */ if (err) { cancelled = true zipfile.close() @@ -71,13 +71,14 @@ module.exports = function (zipPath, opts, cb) { } fs.realpath(destDir, function (err, canonicalDestDir) { + /* istanbul ignore if */ if (err) { cancelled = true zipfile.close() return cb(err) } - var relativeDestDir = path.relative(opts.dir, canonicalDestDir) + const relativeDestDir = path.relative(opts.dir, canonicalDestDir) if (relativeDestDir.split(path.sep).indexOf('..') !== -1) { cancelled = true @@ -100,8 +101,9 @@ module.exports = function (zipPath, opts, cb) { }) function extractEntry (entry, done) { + /* istanbul ignore if */ if (cancelled) { - debug('skipping entry extraction', entry.fileName, {cancelled: cancelled}) + debug('skipping entry extraction', entry.fileName, { cancelled: cancelled }) return setImmediate(done) } @@ -109,16 +111,16 @@ module.exports = function (zipPath, opts, cb) { opts.onEntry(entry, zipfile) } - var dest = path.join(opts.dir, entry.fileName) + const dest = path.join(opts.dir, entry.fileName) // convert external file attr int into a fs stat mode int - var mode = (entry.externalFileAttributes >> 16) & 0xFFFF + let mode = (entry.externalFileAttributes >> 16) & 0xFFFF // check if it's a symlink or dir (using stat mode constants) - var IFMT = 61440 - var IFDIR = 16384 - var IFLNK = 40960 - var symlink = (mode & IFMT) === IFLNK - var isDir = (mode & IFMT) === IFDIR + 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.slice(-1) === '/') { @@ -127,35 +129,35 @@ module.exports = function (zipPath, opts, cb) { // check for windows weird way of specifying a directory // https://github.com/maxogden/extract-zip/issues/13#issuecomment-154494566 - var madeBy = entry.versionMadeBy >> 8 + 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 = 493 // Default to 0755 + if (!mode) mode = 0o755 } else { if (opts.defaultFileMode) mode = parseInt(opts.defaultFileMode, 10) - if (!mode) mode = 420 // Default to 0644 + if (!mode) mode = 0o644 } } debug('extracting entry', { filename: entry.fileName, isDir: isDir, isSymlink: symlink }) // reverse umask first (~) - var umask = ~process.umask() + const umask = ~process.umask() // & with processes umask to override invalid perms - var procMode = mode & umask + const procMode = mode & umask // always ensure folders are created - var destDir = dest - if (!isDir) destDir = path.dirname(dest) + const destDir = isDir ? dest : path.dirname(dest) - debug('mkdirp', {dir: destDir}) - mkdirp(destDir, function (err) { + debug('mkdirp', { dir: destDir }) + fs.mkdir(destDir, { recursive: true }, function (err) { + /* istanbul ignore if */ if (err) { - debug('mkdirp error', destDir, {error: err}) + debug('mkdirp error', destDir, { error: err }) cancelled = true return done(err) } @@ -164,6 +166,7 @@ module.exports = function (zipPath, opts, cb) { debug('opening read stream', dest) zipfile.openReadStream(entry, function (err, readStream) { + /* istanbul ignore if */ if (err) { debug('openReadStream error', err) cancelled = true @@ -171,6 +174,7 @@ module.exports = function (zipPath, opts, cb) { } readStream.on('error', function (err) { + /* istanbul ignore next */ console.log('read err', err) }) @@ -178,15 +182,15 @@ module.exports = function (zipPath, opts, cb) { else writeStream() function writeStream () { - var writeStream = fs.createWriteStream(dest, {mode: procMode}) + const writeStream = fs.createWriteStream(dest, { mode: procMode }) readStream.pipe(writeStream) writeStream.on('finish', function () { done() }) - writeStream.on('error', function (err) { - debug('write error', {error: err}) + writeStream.on('error', /* istanbul ignore next */ function (err) { + debug('write error', { error: err }) cancelled = true return done(err) }) @@ -195,7 +199,7 @@ module.exports = function (zipPath, opts, cb) { // AFAICT the content of the symlink file itself is the symlink target filename string function writeSymlink () { readStream.pipe(concat(function (data) { - var link = data.toString() + const link = data.toString() debug('creating symlink', link, dest) fs.symlink(link, dest, function (err) { if (err) cancelled = true diff --git a/package.json b/package.json index 1e1302d..935410c 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,9 @@ "extract-zip": "cli.js" }, "scripts": { - "test": "standard && node test/test.js" + "coverage": "nyc node test/test.js", + "lint": "standard", + "test": "node test/test.js" }, "files": [ "*.js" @@ -20,17 +22,19 @@ "zip", "extract" ], + "engines": { + "node": ">= 10.12.0" + }, "dependencies": { - "concat-stream": "^1.6.2", - "debug": "^2.6.9", - "mkdirp": "^0.5.4", + "concat-stream": "^2.0.0", + "debug": "^4.1.1", "yauzl": "^2.10.0" }, "devDependencies": { - "rimraf": "^2.2.8", - "standard": "^5.2.2", - "tape": "^4.2.0", - "temp": "^0.8.3" + "nyc": "^15.0.0", + "rimraf": "^3.0.2", + "standard": "^14.3.3", + "tape": "^4.2.0" }, "directories": { "test": "test" diff --git a/readme.md b/readme.md index 403bd58..37f0e04 100644 --- a/readme.md +++ b/readme.md @@ -4,12 +4,14 @@ Unzip written in pure JavaScript. Extracts a zip into a directory. Available as Uses the [`yauzl`](http://npmjs.org/yauzl) ZIP parser. -[![NPM](https://nodei.co/npm/extract-zip.png?global=true)](https://nodei.co/npm/extract-zip/) -[![js-standard-style](https://cdn.rawgit.com/feross/standard/master/badge.svg)](https://github.com/feross/standard) -[![Build Status](https://travis-ci.org/maxogden/extract-zip.svg?branch=master)](https://travis-ci.org/maxogden/extract-zip) +[![NPM](https://nodei.co/npm/extract-zip.png?global=true)](https://npm.im/extract-zip) +[![Uses JS Standard Style](https://cdn.jsdelivr.net/gh/standard/standard/badge.svg)](https://github.com/standard/standard) +[![Build Status](https://github.com/maxogden/extract-zip/workflows/CI/badge.svg)](https://github.com/maxogden/extract-zip/actions?query=workflow%3ACI) ## Installation +Make sure you have Node 10 or greater installed. + Get the library: ``` @@ -26,14 +28,14 @@ npm install extract-zip -g ```js var extract = require('extract-zip') -extract(sourcePath, {dir: targetPath}, function (err) { +extract(source, {dir: target}, function (err) { // extraction is complete. make sure to handle the err }) ``` ### Options -- `dir` - defaults to `process.cwd()` +- `dir` (required) - the path to the directory where the extracted files are written - `defaultDirMode` - integer - Directory Mode (permissions) will default to `493` (octal `0755` in integer) - `defaultFileMode` - integer - File Mode (permissions) will default to `420` (octal `0644` in integer) - `onEntry` - function - if present, will be called with `(entry, zipfile)`, entry is every entry from the zip file forwarded from the `entry` event from yauzl. `zipfile` is the `yauzl` instance diff --git a/test/test.js b/test/test.js index 38a91cc..4f4d105 100644 --- a/test/test.js +++ b/test/test.js @@ -1,22 +1,21 @@ -var extract = require('../') -var fs = require('fs') -var os = require('os') -var path = require('path') -var rimraf = require('rimraf') -var temp = require('temp').track() -var test = require('tape') - -var catsZip = path.join(__dirname, 'cats.zip') -var githubZip = path.join(__dirname, 'github.zip') -var subdirZip = path.join(__dirname, 'file-in-subdir-without-subdir-entry.zip') -var symlinkDestZip = path.join(__dirname, 'symlink-dest.zip') -var symlinkZip = path.join(__dirname, 'symlink.zip') -var brokenZip = path.join(__dirname, 'broken.zip') - -var relativeTarget = './cats' +const extract = require('../') +const fs = require('fs') +const os = require('os') +const path = require('path') +const rimraf = require('rimraf') +const test = require('tape') + +const catsZip = path.join(__dirname, 'cats.zip') +const githubZip = path.join(__dirname, 'github.zip') +const subdirZip = path.join(__dirname, 'file-in-subdir-without-subdir-entry.zip') +const symlinkDestZip = path.join(__dirname, 'symlink-dest.zip') +const symlinkZip = path.join(__dirname, 'symlink.zip') +const brokenZip = path.join(__dirname, 'broken.zip') + +const relativeTarget = './cats' function mkdtemp (t, suffix, callback) { - temp.mkdir({prefix: 'extract-zip', suffix: suffix}, function (err, dirPath) { + fs.mkdtemp(path.join(os.tmpdir(), `extract-zip-${suffix}`), function (err, dirPath) { t.notOk(err, 'no error when creating temporary directory') callback(dirPath) }) @@ -24,7 +23,7 @@ function mkdtemp (t, suffix, callback) { function tempExtract (t, suffix, zipPath, callback) { mkdtemp(t, suffix, function (dirPath) { - extract(zipPath, {dir: dirPath}, function (err) { + extract(zipPath, { dir: dirPath }, function (err) { t.notOk(err, 'no error when extracting ' + zipPath) callback(dirPath) @@ -34,33 +33,43 @@ function tempExtract (t, suffix, zipPath, callback) { function relativeExtract (callback) { rimraf.sync(relativeTarget) - extract(catsZip, {dir: relativeTarget}, callback) + extract(catsZip, { dir: relativeTarget }, callback) rimraf.sync(relativeTarget) } +function exists (t, pathToCheck, message) { + const exists = fs.existsSync(pathToCheck) + t.true(exists, message) +} + +function doesntExist (t, pathToCheck, message) { + const exists = fs.existsSync(pathToCheck) + t.false(exists, message) +} + test('files', function (t) { t.plan(3) tempExtract(t, 'files', catsZip, function (dirPath) { - fs.exists(path.join(dirPath, 'cats', 'gJqEYBs.jpg'), function (exists) { - t.ok(exists, 'file created') - }) + exists(t, path.join(dirPath, 'cats', 'gJqEYBs.jpg'), 'file created') }) }) test('symlinks', function (t) { - t.plan(5) + t.plan(7) tempExtract(t, 'symlinks', catsZip, function (dirPath) { - var symlink = path.join(dirPath, 'cats', 'orange_symlink') + const symlink = path.join(dirPath, 'cats', 'orange_symlink') - fs.exists(symlink, function (exists) { - t.ok(exists, 'symlink created') - }) + exists(t, symlink, 'symlink created') fs.lstat(symlink, function (err, stats) { - t.same(err, null, 'symlink can be stat\'d') + t.same(err, null, "symlink can be stat'd") t.ok(stats.isSymbolicLink(), 'symlink is valid') + fs.readlink(symlink, function (err, linkString) { + t.same(err, null, 'symlink itself can be read') + t.equal(linkString, 'orange') + }) }) }) }) @@ -69,21 +78,17 @@ test('directories', function (t) { t.plan(8) tempExtract(t, 'directories', catsZip, function (dirPath) { - var dirWithContent = path.join(dirPath, 'cats', 'orange') - var dirWithoutContent = path.join(dirPath, 'cats', 'empty') + const dirWithContent = path.join(dirPath, 'cats', 'orange') + const dirWithoutContent = path.join(dirPath, 'cats', 'empty') - fs.exists(dirWithContent, function (exists) { - t.ok(exists, 'directory created') - }) + exists(t, dirWithContent, 'directory created') fs.readdir(dirWithContent, function (err, files) { t.same(err, null, 'directory can be read') t.ok(files.length > 0, 'directory has files') }) - fs.exists(dirWithoutContent, function (exists) { - t.ok(exists, 'empty directory created') - }) + exists(t, dirWithoutContent, 'empty directory created') fs.readdir(dirWithoutContent, function (err, files) { t.same(err, null, 'empty directory can be read') @@ -96,8 +101,27 @@ test('verify github zip extraction worked', function (t) { t.plan(3) tempExtract(t, 'verify-extraction', githubZip, function (dirPath) { - fs.exists(path.join(dirPath, 'extract-zip-master', 'test'), function (exists) { - t.ok(exists, 'folder created') + exists(t, path.join(dirPath, 'extract-zip-master', 'test'), 'folder created') + }) +}) + +test('opts.onEntry', function (t) { + t.plan(3) + + mkdtemp(t, 'onEntry', function (dirPath) { + const actualEntries = [] + const expectedEntries = [ + 'symlink/', + 'symlink/foo.txt', + 'symlink/foo_symlink.txt' + ] + const onEntry = function (entry) { + actualEntries.push(entry.fileName) + } + extract(symlinkZip, { dir: dirPath, onEntry }, function (err) { + t.notOk(err) + + t.same(actualEntries, expectedEntries, 'entries should match') }) }) }) @@ -107,7 +131,7 @@ test('callback called once', function (t) { tempExtract(t, 'callback', symlinkZip, function (dirPath) { // this triggers an error due to symlink creation - extract(symlinkZip, {dir: dirPath}, function (err) { + extract(symlinkZip, { dir: dirPath }, function (err) { if (err) t.ok(true, 'error passed') t.ok(true, 'callback called') @@ -129,69 +153,51 @@ test('no folder created', function (t) { relativeExtract(function (err) { t.true(err instanceof Error, 'is native V8 error') - t.false(fs.existsSync(path.join(__dirname, relativeTarget)), 'file not created') + doesntExist(t, path.join(__dirname, relativeTarget), 'file not created') }) }) -test('symlink destination disallowed', function (t) { - t.plan(4) +if (process.platform !== 'win32') { + test('symlink destination disallowed', function (t) { + t.plan(4) - mkdtemp(t, 'symlink-destination-disallowed', function (dirPath) { - fs.exists(path.join(dirPath, 'file.txt'), function (exists) { - t.false(exists, 'file doesn\'t exist at symlink target') - - extract(symlinkDestZip, {dir: dirPath}, function (err) { - var canonicalTmp = fs.realpathSync(os.tmpdir()) + mkdtemp(t, 'symlink-destination-disallowed', function (dirPath) { + doesntExist(t, path.join(dirPath, 'file.txt'), "file doesn't exist at symlink target") + extract(symlinkDestZip, { dir: dirPath }, function (err) { t.true(err instanceof Error, 'is native V8 error') if (err) { - t.same(err.message, 'Out of bound path "' + canonicalTmp + '" found while processing file symlink-dest/aaa/file.txt', 'has descriptive error message') + t.match(err.message, /Out of bound path ".*?" found while processing file symlink-dest\/aaa\/file.txt/, 'has descriptive error message') } }) }) }) -}) - -test('no file created out of bound', function (t) { - t.plan(7) - mkdtemp(t, 'out-of-bounds-file', function (dirPath) { - extract(symlinkDestZip, {dir: dirPath}, function (err) { - var symlinkDestDir = path.join(dirPath, 'symlink-dest') + test('no file created out of bound', function (t) { + t.plan(7) - t.true(err instanceof Error, 'is native V8 error') - - fs.exists(symlinkDestDir, function (exists) { - t.true(exists, 'target folder created') - }) - - fs.exists(path.join(symlinkDestDir, 'aaa'), function (exists) { - t.true(exists, 'symlink created') - }) + mkdtemp(t, 'out-of-bounds-file', function (dirPath) { + extract(symlinkDestZip, { dir: dirPath }, function (err) { + const symlinkDestDir = path.join(dirPath, 'symlink-dest') - fs.exists(path.join(symlinkDestDir, 'ccc'), function (exists) { - t.true(exists, 'parent folder created') - }) - - fs.exists(path.join(symlinkDestDir, 'ccc/file.txt'), function (exists) { - t.false(exists, 'file not created in original folder') - }) + t.true(err instanceof Error, 'is native V8 error') - fs.exists(path.join(dirPath, 'file.txt'), function (exists) { - t.false(exists, 'file not created in symlink target') + exists(t, symlinkDestDir, 'target folder created') + exists(t, path.join(symlinkDestDir, 'aaa'), 'symlink created') + exists(t, path.join(symlinkDestDir, 'ccc'), 'parent folder created') + doesntExist(t, path.join(symlinkDestDir, 'ccc/file.txt'), 'file not created in original folder') + doesntExist(t, path.join(dirPath, 'file.txt'), 'file not created in symlink target') }) }) }) -}) +} test('files in subdirs where the subdir does not have its own entry is extracted', function (t) { t.plan(3) tempExtract(t, 'subdir-file', subdirZip, function (dirPath) { - fs.exists(path.join(dirPath, 'foo', 'bar'), function (exists) { - t.ok(exists, 'file created') - }) + exists(t, path.join(dirPath, 'foo', 'bar'), 'file created') }) }) @@ -199,7 +205,7 @@ test('extract broken zip', function (t) { t.plan(2) mkdtemp(t, 'broken-zip', function (dirPath) { - extract(brokenZip, {dir: dirPath}, function (err) { + extract(brokenZip, { dir: dirPath }, function (err) { t.ok(err, 'Error: invalid central directory file header signature: 0x2014b00') }) })