From a2701a63de64b0c7aec580659c5458699963874f Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:11:10 -0700 Subject: [PATCH 01/24] build: replace Travis CI with GitHub Actions --- .github/workflows/ci.yml | 41 ++++++++++++++++++++++++++++++++++++++++ .travis.yml | 9 --------- package.json | 3 ++- 3 files changed, 43 insertions(+), 10 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..c1df296 --- /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 test 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/package.json b/package.json index 1e1302d..7d04da9 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,8 @@ "extract-zip": "cli.js" }, "scripts": { - "test": "standard && node test/test.js" + "lint": "standard", + "test": "node test/test.js" }, "files": [ "*.js" From 75d039f15d447b8e2c71bb23d339c48dc9d41716 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:17:16 -0700 Subject: [PATCH 02/24] refactor: remove use of temp module --- package.json | 3 +-- test/test.js | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 7d04da9..625a045 100644 --- a/package.json +++ b/package.json @@ -30,8 +30,7 @@ "devDependencies": { "rimraf": "^2.2.8", "standard": "^5.2.2", - "tape": "^4.2.0", - "temp": "^0.8.3" + "tape": "^4.2.0" }, "directories": { "test": "test" diff --git a/test/test.js b/test/test.js index 38a91cc..3900cf3 100644 --- a/test/test.js +++ b/test/test.js @@ -3,7 +3,6 @@ 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') @@ -16,7 +15,7 @@ var brokenZip = path.join(__dirname, 'broken.zip') var 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) }) From 3cdf195516cb6148519a9fbae1c7a9a4278d8d77 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:20:54 -0700 Subject: [PATCH 03/24] test: don't resolve tmpdir to real path --- test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.js b/test/test.js index 3900cf3..4a2e474 100644 --- a/test/test.js +++ b/test/test.js @@ -140,7 +140,7 @@ test('symlink destination disallowed', function (t) { t.false(exists, 'file doesn\'t exist at symlink target') extract(symlinkDestZip, {dir: dirPath}, function (err) { - var canonicalTmp = fs.realpathSync(os.tmpdir()) + var canonicalTmp = os.tmpdir() t.true(err instanceof Error, 'is native V8 error') From 6e082ce3ecf9acdda6eef6d61dd3749cc3c6f432 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:25:40 -0700 Subject: [PATCH 04/24] test: don't check for the exact tmp dir --- test/test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/test.js b/test/test.js index 4a2e474..68383ab 100644 --- a/test/test.js +++ b/test/test.js @@ -140,12 +140,10 @@ test('symlink destination disallowed', function (t) { t.false(exists, 'file doesn\'t exist at symlink target') extract(symlinkDestZip, {dir: dirPath}, function (err) { - var canonicalTmp = os.tmpdir() - 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') } }) }) From a528bd62943c2b4d0ffbd290690267894b0b943c Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:31:12 -0700 Subject: [PATCH 05/24] test: do not run symlink test on Windows --- test/test.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/test.js b/test/test.js index 68383ab..1583874 100644 --- a/test/test.js +++ b/test/test.js @@ -132,23 +132,25 @@ test('no folder created', function (t) { }) }) -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') + 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) { - t.true(err instanceof Error, 'is native V8 error') + extract(symlinkDestZip, {dir: dirPath}, function (err) { + t.true(err instanceof Error, 'is native V8 error') - if (err) { - t.match(err.message, /Out of bound path ".*?" found while processing file symlink-dest\/aaa\/file.txt/, 'has descriptive error message') - } + if (err) { + 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) From 3ff24f343911ebfa7af4582027070beec8cfa286 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:32:05 -0700 Subject: [PATCH 06/24] feat: drop use of mkdirp BREAKING CHANGE: drop support for Node < 10.12 --- index.js | 7 +++---- package.json | 4 +++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 682d401..787fef5 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,6 @@ 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') @@ -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) { @@ -63,7 +62,7 @@ module.exports = function (zipPath, opts, cb) { var destDir = path.dirname(path.join(opts.dir, entry.fileName)) - mkdirp(destDir, function (err) { + fs.mkdir(destDir, { recursive: true }, function (err) { if (err) { cancelled = true zipfile.close() @@ -153,7 +152,7 @@ module.exports = function (zipPath, opts, cb) { if (!isDir) destDir = path.dirname(dest) debug('mkdirp', {dir: destDir}) - mkdirp(destDir, function (err) { + fs.mkdir(destDir, { recursive: true }, function (err) { if (err) { debug('mkdirp error', destDir, {error: err}) cancelled = true diff --git a/package.json b/package.json index 625a045..fc1a545 100644 --- a/package.json +++ b/package.json @@ -21,10 +21,12 @@ "zip", "extract" ], + "engines": { + "node": ">= 10.12.0" + }, "dependencies": { "concat-stream": "^1.6.2", "debug": "^2.6.9", - "mkdirp": "^0.5.4", "yauzl": "^2.10.0" }, "devDependencies": { From 1c8252ad1c637d18f7a03a6b2ed8aabc7a005e02 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:38:36 -0700 Subject: [PATCH 07/24] docs: update badges --- readme.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readme.md b/readme.md index 403bd58..b0048ab 100644 --- a/readme.md +++ b/readme.md @@ -4,9 +4,9 @@ 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 From 42780ca76de13941583d86942b74a637c1097dd3 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:40:43 -0700 Subject: [PATCH 08/24] build(deps): upgrade debug to ^4.1.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fc1a545..1790097 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ }, "dependencies": { "concat-stream": "^1.6.2", - "debug": "^2.6.9", + "debug": "^4.1.1", "yauzl": "^2.10.0" }, "devDependencies": { From aa8b83d75edbc220afbdf81002c14efe88890b1b Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:42:24 -0700 Subject: [PATCH 09/24] build(deps-dev): upgrade rimraf to ^3.0.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1790097..648edd0 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "yauzl": "^2.10.0" }, "devDependencies": { - "rimraf": "^2.2.8", + "rimraf": "^3.0.2", "standard": "^5.2.2", "tape": "^4.2.0" }, From c46e3f9d2dadbbe6eeb7285ed1d26fc0cec7f918 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:43:43 -0700 Subject: [PATCH 10/24] build(deps-dev): upgrade standard to ^14.3.3 --- cli.js | 2 +- index.js | 14 +++++++------- package.json | 2 +- test/test.js | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) 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 787fef5..00f1a0d 100644 --- a/index.js +++ b/index.js @@ -26,7 +26,7 @@ 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 @@ -48,7 +48,7 @@ module.exports = function (zipPath, opts, cb) { zipfile.on('entry', function (entry) { if (cancelled) { - debug('skipping entry', entry.fileName, {cancelled: cancelled}) + debug('skipping entry', entry.fileName, { cancelled: cancelled }) return } @@ -100,7 +100,7 @@ module.exports = function (zipPath, opts, cb) { function extractEntry (entry, done) { if (cancelled) { - debug('skipping entry extraction', entry.fileName, {cancelled: cancelled}) + debug('skipping entry extraction', entry.fileName, { cancelled: cancelled }) return setImmediate(done) } @@ -151,10 +151,10 @@ module.exports = function (zipPath, opts, cb) { var destDir = dest if (!isDir) destDir = path.dirname(dest) - debug('mkdirp', {dir: destDir}) + debug('mkdirp', { dir: destDir }) fs.mkdir(destDir, { recursive: true }, function (err) { if (err) { - debug('mkdirp error', destDir, {error: err}) + debug('mkdirp error', destDir, { error: err }) cancelled = true return done(err) } @@ -177,7 +177,7 @@ module.exports = function (zipPath, opts, cb) { else writeStream() function writeStream () { - var writeStream = fs.createWriteStream(dest, {mode: procMode}) + var writeStream = fs.createWriteStream(dest, { mode: procMode }) readStream.pipe(writeStream) writeStream.on('finish', function () { @@ -185,7 +185,7 @@ module.exports = function (zipPath, opts, cb) { }) writeStream.on('error', function (err) { - debug('write error', {error: err}) + debug('write error', { error: err }) cancelled = true return done(err) }) diff --git a/package.json b/package.json index 648edd0..1bbd6f9 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ }, "devDependencies": { "rimraf": "^3.0.2", - "standard": "^5.2.2", + "standard": "^14.3.3", "tape": "^4.2.0" }, "directories": { diff --git a/test/test.js b/test/test.js index 1583874..0886a7d 100644 --- a/test/test.js +++ b/test/test.js @@ -23,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) @@ -33,7 +33,7 @@ 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) } @@ -106,7 +106,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') @@ -140,7 +140,7 @@ if (process.platform !== 'win32') { 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) { + extract(symlinkDestZip, { dir: dirPath }, function (err) { t.true(err instanceof Error, 'is native V8 error') if (err) { @@ -156,7 +156,7 @@ 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) { + extract(symlinkDestZip, { dir: dirPath }, function (err) { var symlinkDestDir = path.join(dirPath, 'symlink-dest') t.true(err instanceof Error, 'is native V8 error') @@ -198,7 +198,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') }) }) From de3c72c15ef7643aac411caa4a9e5bee42a4ec6f Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:53:17 -0700 Subject: [PATCH 11/24] refactor: remove use of deprecated fs.exists --- test/test.js | 74 ++++++++++++++++++++-------------------------------- 1 file changed, 28 insertions(+), 46 deletions(-) diff --git a/test/test.js b/test/test.js index 0886a7d..fa7ae99 100644 --- a/test/test.js +++ b/test/test.js @@ -37,13 +37,21 @@ function relativeExtract (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') }) }) @@ -53,9 +61,7 @@ test('symlinks', function (t) { tempExtract(t, 'symlinks', catsZip, function (dirPath) { var 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') @@ -71,18 +77,14 @@ test('directories', function (t) { var dirWithContent = path.join(dirPath, 'cats', 'orange') var 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') @@ -95,9 +97,7 @@ 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') }) }) @@ -128,7 +128,7 @@ 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') }) }) @@ -137,16 +137,14 @@ if (process.platform !== 'win32') { 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') + 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') + extract(symlinkDestZip, { dir: dirPath }, function (err) { + t.true(err instanceof Error, 'is native V8 error') - if (err) { - t.match(err.message, /Out of bound path ".*?" found while processing file symlink-dest\/aaa\/file.txt/, 'has descriptive error message') - } - }) + if (err) { + t.match(err.message, /Out of bound path ".*?" found while processing file symlink-dest\/aaa\/file.txt/, 'has descriptive error message') + } }) }) }) @@ -161,25 +159,11 @@ test('no file created out of bound', function (t) { 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') - }) - - 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') - }) - - 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') }) }) }) @@ -188,9 +172,7 @@ test('files in subdirs where the subdir does not have its own entry is extracted 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') }) }) From 944af5bca87abf0e0691b1a172940649fd493fe6 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 18:55:37 -0700 Subject: [PATCH 12/24] build(deps): upgrade concat-stream to ^2.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1bbd6f9..b5eb557 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "node": ">= 10.12.0" }, "dependencies": { - "concat-stream": "^1.6.2", + "concat-stream": "^2.0.0", "debug": "^4.1.1", "yauzl": "^2.10.0" }, From c207520ff9f4614eac302351db4b751776e05b3e Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:02:55 -0700 Subject: [PATCH 13/24] test: add nyc for code coverage --- .github/workflows/ci.yml | 2 +- .gitignore | 1 + index.js | 9 ++++++++- package.json | 2 ++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1df296..09537fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,4 +38,4 @@ jobs: - name: Lint run: yarn lint - name: Test - run: yarn 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/index.js b/index.js index 00f1a0d..124628d 100644 --- a/index.js +++ b/index.js @@ -47,6 +47,7 @@ module.exports = function (zipPath, opts, cb) { }) zipfile.on('entry', function (entry) { + /* istanbul ignore if */ if (cancelled) { debug('skipping entry', entry.fileName, { cancelled: cancelled }) return @@ -63,6 +64,7 @@ module.exports = function (zipPath, opts, cb) { var destDir = path.dirname(path.join(opts.dir, entry.fileName)) fs.mkdir(destDir, { recursive: true }, function (err) { + /* istanbul ignore if */ if (err) { cancelled = true zipfile.close() @@ -70,6 +72,7 @@ module.exports = function (zipPath, opts, cb) { } fs.realpath(destDir, function (err, canonicalDestDir) { + /* istanbul ignore if */ if (err) { cancelled = true zipfile.close() @@ -99,6 +102,7 @@ module.exports = function (zipPath, opts, cb) { }) function extractEntry (entry, done) { + /* istanbul ignore if */ if (cancelled) { debug('skipping entry extraction', entry.fileName, { cancelled: cancelled }) return setImmediate(done) @@ -153,6 +157,7 @@ module.exports = function (zipPath, opts, cb) { debug('mkdirp', { dir: destDir }) fs.mkdir(destDir, { recursive: true }, function (err) { + /* istanbul ignore if */ if (err) { debug('mkdirp error', destDir, { error: err }) cancelled = true @@ -163,6 +168,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 @@ -170,6 +176,7 @@ module.exports = function (zipPath, opts, cb) { } readStream.on('error', function (err) { + /* istanbul ignore next */ console.log('read err', err) }) @@ -184,7 +191,7 @@ module.exports = function (zipPath, opts, cb) { done() }) - writeStream.on('error', function (err) { + writeStream.on('error', /* istanbul ignore next */ function (err) { debug('write error', { error: err }) cancelled = true return done(err) diff --git a/package.json b/package.json index b5eb557..935410c 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "extract-zip": "cli.js" }, "scripts": { + "coverage": "nyc node test/test.js", "lint": "standard", "test": "node test/test.js" }, @@ -30,6 +31,7 @@ "yauzl": "^2.10.0" }, "devDependencies": { + "nyc": "^15.0.0", "rimraf": "^3.0.2", "standard": "^14.3.3", "tape": "^4.2.0" From 4b7465f8399aa819cf5c1f7f6cf53059cf59d55a Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:19:28 -0700 Subject: [PATCH 14/24] test: add test for onEntry option --- test/test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/test.js b/test/test.js index fa7ae99..0774655 100644 --- a/test/test.js +++ b/test/test.js @@ -101,6 +101,27 @@ test('verify github zip extraction worked', function (t) { }) }) +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') + }) + }) +}) + test('callback called once', function (t) { t.plan(4) From d5bc940a14cf961d4d7127eea62ee693933a9e23 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:28:08 -0700 Subject: [PATCH 15/24] test: disable another symlink test on win32 --- test/test.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/test.js b/test/test.js index 0774655..a8f5a93 100644 --- a/test/test.js +++ b/test/test.js @@ -169,25 +169,25 @@ if (process.platform !== 'win32') { }) }) }) -} -test('no file created out of bound', function (t) { - t.plan(7) + 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') + mkdtemp(t, 'out-of-bounds-file', function (dirPath) { + extract(symlinkDestZip, { dir: dirPath }, function (err) { + var symlinkDestDir = path.join(dirPath, 'symlink-dest') - t.true(err instanceof Error, 'is native V8 error') + t.true(err instanceof Error, 'is native V8 error') - 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') + 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) From 49368f50a7e37fce22f6b6aca208164d8f0f1802 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:33:28 -0700 Subject: [PATCH 16/24] refactor: use const/let instead of var --- index.js | 43 +++++++++++++++++++++---------------------- test/test.js | 38 +++++++++++++++++++------------------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/index.js b/index.js index 124628d..ea24e51 100644 --- a/index.js +++ b/index.js @@ -1,8 +1,8 @@ -var fs = require('fs') -var path = require('path') -var yauzl = require('yauzl') -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) @@ -29,7 +29,7 @@ module.exports = function (zipPath, opts, cb) { 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) { @@ -61,7 +61,7 @@ module.exports = function (zipPath, opts, cb) { return } - var destDir = path.dirname(path.join(opts.dir, entry.fileName)) + const destDir = path.dirname(path.join(opts.dir, entry.fileName)) fs.mkdir(destDir, { recursive: true }, function (err) { /* istanbul ignore if */ @@ -79,7 +79,7 @@ module.exports = function (zipPath, opts, cb) { 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 @@ -112,16 +112,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) === '/') { @@ -130,7 +130,7 @@ 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 @@ -147,13 +147,12 @@ module.exports = function (zipPath, opts, cb) { 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 }) fs.mkdir(destDir, { recursive: true }, function (err) { @@ -184,7 +183,7 @@ 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 () { @@ -201,7 +200,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/test/test.js b/test/test.js index a8f5a93..79813ee 100644 --- a/test/test.js +++ b/test/test.js @@ -1,18 +1,18 @@ -var extract = require('../') -var fs = require('fs') -var os = require('os') -var path = require('path') -var rimraf = require('rimraf') -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) { fs.mkdtemp(path.join(os.tmpdir(), `extract-zip-${suffix}`), function (err, dirPath) { @@ -59,7 +59,7 @@ test('symlinks', function (t) { t.plan(5) tempExtract(t, 'symlinks', catsZip, function (dirPath) { - var symlink = path.join(dirPath, 'cats', 'orange_symlink') + const symlink = path.join(dirPath, 'cats', 'orange_symlink') exists(t, symlink, 'symlink created') @@ -74,8 +74,8 @@ 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') exists(t, dirWithContent, 'directory created') @@ -175,7 +175,7 @@ if (process.platform !== 'win32') { mkdtemp(t, 'out-of-bounds-file', function (dirPath) { extract(symlinkDestZip, { dir: dirPath }, function (err) { - var symlinkDestDir = path.join(dirPath, 'symlink-dest') + const symlinkDestDir = path.join(dirPath, 'symlink-dest') t.true(err instanceof Error, 'is native V8 error') From 7840e43c8b6cb68d49e89893934cf3dcee1b7b5e Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:35:54 -0700 Subject: [PATCH 17/24] refactor: use octal literals --- index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index ea24e51..161b22c 100644 --- a/index.js +++ b/index.js @@ -137,10 +137,10 @@ module.exports = function (zipPath, opts, cb) { 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 } } From 6a4e86e42a9c99ad51c7921abb6e1b6995111afc Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:37:52 -0700 Subject: [PATCH 18/24] refactor: use String.prototype.startsWith --- index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/index.js b/index.js index 161b22c..915f2ca 100644 --- a/index.js +++ b/index.js @@ -55,8 +55,7 @@ module.exports = function (zipPath, opts, cb) { debug('zipfile entry', entry.fileName) - if (/^__MACOSX\//.test(entry.fileName)) { - // dir name starts with __MACOSX/ + if (entry.fileName.startsWith('__MACOSX/')) { zipfile.readEntry() return } From b8121dc8db0d94db3ddee7e2b53b423af24eed1f Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:45:00 -0700 Subject: [PATCH 19/24] test: test the symlink's link --- test/test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/test.js b/test/test.js index 79813ee..7b3341e 100644 --- a/test/test.js +++ b/test/test.js @@ -56,7 +56,7 @@ test('files', function (t) { }) test('symlinks', function (t) { - t.plan(5) + t.plan(6) tempExtract(t, 'symlinks', catsZip, function (dirPath) { const symlink = path.join(dirPath, 'cats', 'orange_symlink') @@ -64,8 +64,11 @@ test('symlinks', function (t) { 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.equal(linkString, 'orange') + }) }) }) }) From 50280ac84351e920d5a7e71143dd4df706e70a27 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:49:35 -0700 Subject: [PATCH 20/24] test: assert readlink error doesn't exist --- test/test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test.js b/test/test.js index 7b3341e..a0df9d4 100644 --- a/test/test.js +++ b/test/test.js @@ -67,6 +67,7 @@ test('symlinks', function (t) { 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') }) }) From 2589902b4d4dc2d2e5031e54328db9b3d65c8ef9 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:53:22 -0700 Subject: [PATCH 21/24] chore: fix lint warning --- test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.js b/test/test.js index a0df9d4..8e8d69d 100644 --- a/test/test.js +++ b/test/test.js @@ -67,7 +67,7 @@ test('symlinks', function (t) { 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.same(err, null, 'symlink itself can be read') t.equal(linkString, 'orange') }) }) From 6c95b13fdb329019cf031d0604e5cb1957222a5c Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 19:56:39 -0700 Subject: [PATCH 22/24] test: fix plan --- test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.js b/test/test.js index 8e8d69d..4f4d105 100644 --- a/test/test.js +++ b/test/test.js @@ -56,7 +56,7 @@ test('files', function (t) { }) test('symlinks', function (t) { - t.plan(6) + t.plan(7) tempExtract(t, 'symlinks', catsZip, function (dirPath) { const symlink = path.join(dirPath, 'cats', 'orange_symlink') From a2b80d1ff85d54c50828565f93aa029d2dafc0d3 Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 20:07:10 -0700 Subject: [PATCH 23/24] docs: note Node version requirement in the readme --- readme.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readme.md b/readme.md index b0048ab..4e2ad5c 100644 --- a/readme.md +++ b/readme.md @@ -10,6 +10,8 @@ Uses the [`yauzl`](http://npmjs.org/yauzl) ZIP parser. ## Installation +Make sure you have Node 10 or greater installed. + Get the library: ``` From e5fdad45404be6ff211e9ae466783bef392e10be Mon Sep 17 00:00:00 2001 From: Mark Lee Date: Tue, 24 Mar 2020 20:21:23 -0700 Subject: [PATCH 24/24] docs: fix dir description Closes #61. --- readme.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 4e2ad5c..37f0e04 100644 --- a/readme.md +++ b/readme.md @@ -28,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