From 336fa8f27c44bec70d46a6482096af24c668ee16 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Mon, 19 Jun 2023 21:14:39 -0700 Subject: [PATCH] refactor: dry and other pr comments PR-URL: https://github.com/isaacs/node-tar/pull/391 Credit: @JamieMagee Close: #391 Reviewed-by: @isaacs --- lib/pack.js | 30 ++++++++++++++++-------------- lib/parse.js | 22 ++++------------------ test/extract.js | 21 +++++++++++++++++---- test/pack.js | 6 ++++++ test/parse.js | 22 ++++++++++++++++++++++ 5 files changed, 65 insertions(+), 36 deletions(-) diff --git a/lib/pack.js b/lib/pack.js index 81bc10f3..789a2a71 100644 --- a/lib/pack.js +++ b/lib/pack.js @@ -79,23 +79,25 @@ const Pack = warner(class Pack extends Minipass { this.portable = !!opt.portable this.zip = null - if (opt.gzip) { - if (typeof opt.gzip !== 'object') { - opt.gzip = {} + if (opt.gzip || opt.brotli) { + if (opt.gzip && opt.brotli) { + throw new TypeError('gzip and brotli are mutually exclusive') } - if (this.portable) { - opt.gzip.portable = true + if (opt.gzip) { + if (typeof opt.gzip !== 'object') { + opt.gzip = {} + } + if (this.portable) { + opt.gzip.portable = true + } + this.zip = new zlib.Gzip(opt.gzip) } - this.zip = new zlib.Gzip(opt.gzip) - this.zip.on('data', chunk => super.write(chunk)) - this.zip.on('end', _ => super.end()) - this.zip.on('drain', _ => this[ONDRAIN]()) - this.on('resume', _ => this.zip.resume()) - } else if (opt.brotli) { - if (typeof opt.brotli !== 'object') { - opt.brotli = {} + if (opt.brotli) { + if (typeof opt.brotli !== 'object') { + opt.brotli = {} + } + this.zip = new zlib.BrotliCompress(opt.brotli) } - this.zip = new zlib.BrotliCompress(opt.brotli) this.zip.on('data', chunk => super.write(chunk)) this.zip.on('end', _ => super.end()) this.zip.on('drain', _ => this[ONDRAIN]()) diff --git a/lib/parse.js b/lib/parse.js index 8b8c8cee..6906d059 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -99,7 +99,8 @@ module.exports = warner(class Parser extends EE { this.filter = typeof opt.filter === 'function' ? opt.filter : noop // Unlike gzip, brotli doesn't have any magic bytes to identify it // Users need to explicitly tell us they're extracting a brotli file - this.brotli = opt.brotli + // Or we infer from the file extension + this.brotli = opt.brotli || (opt.file && (opt.file.endsWith('.tar.br') || opt.file.endsWith('.tbr'))) // have to set this so that streams are ok piping into it this.writable = true @@ -359,30 +360,15 @@ module.exports = warner(class Parser extends EE { this[BUFFER] = chunk return true } - if (this[UNZIP] === null && this.brotli) { - const ended = this[ENDED] - this[ENDED] = false - this[UNZIP] = new zlib.BrotliDecompress() - this[UNZIP].on('data', chunk => this[CONSUMECHUNK](chunk)) - this[UNZIP].on('error', er => this.abort(er)) - this[UNZIP].on('end', _ => { - this[ENDED] = true - this[CONSUMECHUNK]() - }) - this[WRITING] = true - const ret = this[UNZIP][ended ? 'end' : 'write'](chunk) - this[WRITING] = false - return ret - } for (let i = 0; this[UNZIP] === null && i < gzipHeader.length; i++) { if (chunk[i] !== gzipHeader[i]) { this[UNZIP] = false } } - if (this[UNZIP] === null) { + if (this[UNZIP] === null || (this[UNZIP] === false && this.brotli)) { const ended = this[ENDED] this[ENDED] = false - this[UNZIP] = new zlib.Unzip() + this[UNZIP] = this.brotli ? new zlib.BrotliDecompress() : new zlib.Unzip() this[UNZIP].on('data', chunk => this[CONSUMECHUNK](chunk)) this[UNZIP].on('error', er => this.abort(er)) this[UNZIP].on('end', _ => { diff --git a/test/extract.js b/test/extract.js index e580db97..c11d0afc 100644 --- a/test/extract.js +++ b/test/extract.js @@ -323,12 +323,25 @@ t.test('brotli', async t => { await rimraf(dir) }) - t.test('fails if brotli', async t => { - const expect = new Error("TAR_BAD_ARCHIVE: Unrecognized archive format") - t.throws(_ => x({ sync: true, file: file }), expect) + t.test('fails if unknown file extension', async t => { + const filename = path.resolve(__dirname, 'brotli/example.unknown') + const f = fs.openSync(filename, 'a') + fs.closeSync(f) + + const expect = new Error('TAR_BAD_ARCHIVE: Unrecognized archive format') + + t.throws(_ => x({ sync: true, file: filename }), expect) + }) + + t.test('succeeds based on file extension', t => { + x({ sync: true, file: file, C: dir }) + + t.same(fs.readdirSync(dir + '/x').sort(), + ['1', '10', '2', '3', '4', '5', '6', '7', '8', '9']) + t.end() }) - t.test('succeeds', t => { + t.test('succeeds when passed explicit option', t => { x({ sync: true, file: file, C: dir, brotli: true }) t.same(fs.readdirSync(dir + '/x').sort(), diff --git a/test/pack.js b/test/pack.js index 99c733d3..a4f8bfbe 100644 --- a/test/pack.js +++ b/test/pack.js @@ -382,6 +382,12 @@ t.test('if brotli is truthy, make it an object', t => { t.end() }) +t.test('throws if both gzip and brotli are truthy', t => { + const opt = { gzip: true, brotli: true } + t.throws(_ => new Pack(opt), new TypeError('gzip and brotli are mutually exclusive')) + t.end() +}) + t.test('gzip, also a very deep path', t => { const out = [] diff --git a/test/parse.js b/test/parse.js index dd74b2c7..dff01f3c 100644 --- a/test/parse.js +++ b/test/parse.js @@ -125,6 +125,28 @@ t.test('fixture tests', t => { bs.end(zlib.gzipSync(tardata)) }) + t.test('compress with brotli based on filename .tar.br', t => { + const p = new Parse({ + maxMetaEntrySize: maxMeta, + filter: filter ? (path, entry) => entry.size % 2 !== 0 : null, + strict: strict, + file: 'example.tar.br', + }) + trackEvents(t, expect, p) + p.end(zlib.brotliCompressSync(tardata)) + }) + + t.test('compress with brotli based on filename .tbr', t => { + const p = new Parse({ + maxMetaEntrySize: maxMeta, + filter: filter ? (path, entry) => entry.size % 2 !== 0 : null, + strict: strict, + file: 'example.tbr', + }) + trackEvents(t, expect, p) + p.end(zlib.brotliCompressSync(tardata)) + }) + t.test('compress with brotli all at once', t => { const p = new Parse({ maxMetaEntrySize: maxMeta,