From 91383e47fdbb87ae45365396dd0150dcad8ed967 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Wed, 22 Mar 2017 19:45:03 -0700 Subject: [PATCH] zlib: support Uint8Array in convenience methods Also support Uint8Array as a `dictionary` option. PR-URL: https://github.com/nodejs/node/pull/12001 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca --- doc/api/zlib.md | 139 ++++++++++++++---- lib/zlib.js | 19 ++- .../parallel/test-zlib-convenience-methods.js | 81 +++++----- .../test-zlib-deflate-constructors.js | 2 +- test/parallel/test-zlib-dictionary.js | 19 ++- .../test-zlib-not-string-or-buffer.js | 3 +- 6 files changed, 176 insertions(+), 87 deletions(-) diff --git a/doc/api/zlib.md b/doc/api/zlib.md index 5b02a8ed37d278..928070d5778ca7 100644 --- a/doc/api/zlib.md +++ b/doc/api/zlib.md @@ -278,6 +278,9 @@ Compression strategy. -All of these take a [Buffer][] or string as the first argument, an optional -second argument to supply options to the `zlib` classes and will call the -supplied callback with `callback(error, result)`. +All of these take a [Buffer][], [Uint8Array][], or string as the first +argument, an optional second argument to supply options to the `zlib` classes +and will call the supplied callback with `callback(error, result)`. Every method has a `*Sync` counterpart, which accept the same arguments, but without a callback. -### zlib.deflate(buf[, options], callback) +### zlib.deflate(buffer[, options], callback) -### zlib.deflateSync(buf[, options]) +### zlib.deflateSync(buffer[, options]) -Compress a [Buffer][] or string with [Deflate][]. +- `buffer` {Buffer|Uint8Array|string} + +Compress a chunk of data with [Deflate][]. -### zlib.deflateRaw(buf[, options], callback) +### zlib.deflateRaw(buffer[, options], callback) -### zlib.deflateRawSync(buf[, options]) +### zlib.deflateRawSync(buffer[, options]) -Compress a [Buffer][] or string with [DeflateRaw][]. +- `buffer` {Buffer|Uint8Array|string} + +Compress a chunk of data with [DeflateRaw][]. -### zlib.gunzip(buf[, options], callback) +### zlib.gunzip(buffer[, options], callback) -### zlib.gunzipSync(buf[, options]) +### zlib.gunzipSync(buffer[, options]) -Decompress a [Buffer][] or string with [Gunzip][]. +- `buffer` {Buffer|Uint8Array|string} + +Decompress a chunk of data with [Gunzip][]. -### zlib.gzip(buf[, options], callback) +### zlib.gzip(buffer[, options], callback) -### zlib.gzipSync(buf[, options]) +### zlib.gzipSync(buffer[, options]) -Compress a [Buffer][] or string with [Gzip][]. +- `buffer` {Buffer|Uint8Array|string} -### zlib.inflate(buf[, options], callback) +Compress a chunk of data with [Gzip][]. + +### zlib.inflate(buffer[, options], callback) -### zlib.inflateSync(buf[, options]) +### zlib.inflateSync(buffer[, options]) -Decompress a [Buffer][] or string with [Inflate][]. +- `buffer` {Buffer|Uint8Array|string} -### zlib.inflateRaw(buf[, options], callback) +Decompress a chunk of data with [Inflate][]. + +### zlib.inflateRaw(buffer[, options], callback) -### zlib.inflateRawSync(buf[, options]) +### zlib.inflateRawSync(buffer[, options]) -Decompress a [Buffer][] or string with [InflateRaw][]. +- `buffer` {Buffer|Uint8Array|string} + +Decompress a chunk of data with [InflateRaw][]. -### zlib.unzip(buf[, options], callback) +### zlib.unzip(buffer[, options], callback) -### zlib.unzipSync(buf[, options]) +### zlib.unzipSync(buffer[, options]) -Decompress a [Buffer][] or string with [Unzip][]. +- `buffer` {Buffer|Uint8Array|string} + +Decompress a chunk of data with [Unzip][]. [`Accept-Encoding`]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.3 [`Content-Encoding`]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.11 @@ -571,3 +645,4 @@ Decompress a [Buffer][] or string with [Unzip][]. [Unzip]: #zlib_class_zlib_unzip [`.flush()`]: #zlib_zlib_flush_kind_callback [Buffer]: buffer.html +[Uint8Array]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array diff --git a/lib/zlib.js b/lib/zlib.js index ab06c736893394..07040c3ebc581e 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -24,6 +24,7 @@ const Buffer = require('buffer').Buffer; const internalUtil = require('internal/util'); const Transform = require('_stream_transform'); +const { isUint8Array } = process.binding('util'); const binding = process.binding('zlib'); const assert = require('assert').ok; const kMaxLength = require('buffer').kMaxLength; @@ -78,6 +79,13 @@ function isInvalidStrategy(strategy) { } function zlibBuffer(engine, buffer, callback) { + // Streams do not support non-Buffer Uint8Arrays yet. Convert it to a + // Buffer without copying. + if (isUint8Array(buffer) && + Object.getPrototypeOf(buffer) !== Buffer.prototype) { + buffer = Buffer.from(buffer.buffer, buffer.byteOffset, buffer.byteLength); + } + var buffers = []; var nread = 0; @@ -121,8 +129,9 @@ function zlibBuffer(engine, buffer, callback) { function zlibBufferSync(engine, buffer) { if (typeof buffer === 'string') buffer = Buffer.from(buffer); - if (!(buffer instanceof Buffer)) - throw new TypeError('Not a string or buffer'); + else if (!isUint8Array(buffer)) + throw new TypeError('"buffer" argument must be a string, Buffer, or ' + + 'Uint8Array'); var flushFlag = engine._finishFlushFlag; @@ -205,9 +214,9 @@ class Zlib extends Transform { throw new TypeError('Invalid strategy: ' + opts.strategy); if (opts.dictionary) { - if (!(opts.dictionary instanceof Buffer)) { + if (!isUint8Array(opts.dictionary)) { throw new TypeError( - 'Invalid dictionary: it should be a Buffer instance'); + 'Invalid dictionary: it should be a Buffer or an Uint8Array'); } } @@ -302,7 +311,7 @@ class Zlib extends Transform { var ending = ws.ending || ws.ended; var last = ending && (!chunk || ws.length === chunk.length); - if (chunk !== null && !(chunk instanceof Buffer)) + if (chunk !== null && !isUint8Array(chunk)) return cb(new TypeError('invalid input')); if (!this._handle) diff --git a/test/parallel/test-zlib-convenience-methods.js b/test/parallel/test-zlib-convenience-methods.js index 5f1e1ce9e20b0b..df56f21ff49529 100644 --- a/test/parallel/test-zlib-convenience-methods.js +++ b/test/parallel/test-zlib-convenience-methods.js @@ -22,59 +22,60 @@ 'use strict'; // test convenience methods with and without options supplied -require('../common'); +const common = require('../common'); const assert = require('assert'); const zlib = require('zlib'); -let hadRun = 0; - -const expect = 'blahblahblahblahblahblah'; +const expectStr = 'blahblahblahblahblahblah'; +const expectBuf = Buffer.from(expectStr); +const expectUint8Array = new Uint8Array(expectBuf); const opts = { level: 9, chunkSize: 1024, }; -[ +for (const method of [ ['gzip', 'gunzip'], ['gzip', 'unzip'], ['deflate', 'inflate'], ['deflateRaw', 'inflateRaw'], -].forEach(function(method) { - - zlib[method[0]](expect, opts, function(err, result) { - zlib[method[1]](result, opts, function(err, result) { - assert.strictEqual(result.toString(), expect, - 'Should get original string after ' + - method[0] + '/' + method[1] + ' with options.'); - hadRun++; - }); - }); - - zlib[method[0]](expect, function(err, result) { - zlib[method[1]](result, function(err, result) { - assert.strictEqual(result.toString(), expect, - 'Should get original string after ' + - method[0] + '/' + method[1] + ' without options.'); - hadRun++; - }); - }); +]) { + for (const [type, expect] of [ + ['string', expectStr], + ['Buffer', expectBuf], + ['Uint8Array', expectUint8Array] + ]) { + zlib[method[0]](expect, opts, common.mustCall((err, result) => { + zlib[method[1]](result, opts, common.mustCall((err, result) => { + assert.strictEqual(result.toString(), expectStr, + `Should get original string after ${method[0]}/` + + `${method[1]} ${type} with options.`); + })); + })); - let result = zlib[method[0] + 'Sync'](expect, opts); - result = zlib[method[1] + 'Sync'](result, opts); - assert.strictEqual(result.toString(), expect, - 'Should get original string after ' + - method[0] + '/' + method[1] + ' with options.'); - hadRun++; + zlib[method[0]](expect, common.mustCall((err, result) => { + zlib[method[1]](result, common.mustCall((err, result) => { + assert.strictEqual(result.toString(), expectStr, + `Should get original string after ${method[0]}/` + + `${method[1]} ${type} without options.`); + })); + })); - result = zlib[method[0] + 'Sync'](expect); - result = zlib[method[1] + 'Sync'](result); - assert.strictEqual(result.toString(), expect, - 'Should get original string after ' + - method[0] + '/' + method[1] + ' without options.'); - hadRun++; + { + const compressed = zlib[method[0] + 'Sync'](expect, opts); + const decompressed = zlib[method[1] + 'Sync'](compressed, opts); + assert.strictEqual(decompressed.toString(), expectStr, + `Should get original string after ${method[0]}Sync/` + + `${method[1]}Sync ${type} with options.`); + } -}); -process.on('exit', function() { - assert.strictEqual(hadRun, 16, 'expect 16 compressions'); -}); + { + const compressed = zlib[method[0] + 'Sync'](expect); + const decompressed = zlib[method[1] + 'Sync'](compressed); + assert.strictEqual(decompressed.toString(), expectStr, + `Should get original string after ${method[0]}Sync/` + + `${method[1]}Sync ${type} without options.`); + } + } +} diff --git a/test/parallel/test-zlib-deflate-constructors.js b/test/parallel/test-zlib-deflate-constructors.js index ca9c4f801b1a4c..c495d2d11d5acb 100644 --- a/test/parallel/test-zlib-deflate-constructors.js +++ b/test/parallel/test-zlib-deflate-constructors.js @@ -107,5 +107,5 @@ assert.throws( // Throws if opts.dictionary is not a Buffer assert.throws( () => { new zlib.Deflate({dictionary: 'not a buffer'}); }, - /^TypeError: Invalid dictionary: it should be a Buffer instance$/ + /^TypeError: Invalid dictionary: it should be a Buffer or an Uint8Array$/ ); diff --git a/test/parallel/test-zlib-dictionary.js b/test/parallel/test-zlib-dictionary.js index fd70d73556d0a9..a3b55bc72d4fc6 100644 --- a/test/parallel/test-zlib-dictionary.js +++ b/test/parallel/test-zlib-dictionary.js @@ -41,6 +41,7 @@ const spdyDict = Buffer.from([ 'ation/xhtmltext/plainpublicmax-agecharset=iso-8859-1utf-8gzipdeflateHTTP/1', '.1statusversionurl\0' ].join('')); +const spdyDictUint8Array = new Uint8Array(spdyDict); const input = [ 'HTTP/1.1 200 Ok', @@ -49,7 +50,7 @@ const input = [ '' ].join('\r\n'); -function basicDictionaryTest() { +function basicDictionaryTest(spdyDict) { let output = ''; const deflate = zlib.createDeflate({ dictionary: spdyDict }); const inflate = zlib.createInflate({ dictionary: spdyDict }); @@ -75,7 +76,7 @@ function basicDictionaryTest() { deflate.end(); } -function deflateResetDictionaryTest() { +function deflateResetDictionaryTest(spdyDict) { let doneReset = false; let output = ''; const deflate = zlib.createDeflate({ dictionary: spdyDict }); @@ -108,7 +109,7 @@ function deflateResetDictionaryTest() { }); } -function rawDictionaryTest() { +function rawDictionaryTest(spdyDict) { let output = ''; const deflate = zlib.createDeflateRaw({ dictionary: spdyDict }); const inflate = zlib.createInflateRaw({ dictionary: spdyDict }); @@ -134,7 +135,7 @@ function rawDictionaryTest() { deflate.end(); } -function deflateRawResetDictionaryTest() { +function deflateRawResetDictionaryTest(spdyDict) { let doneReset = false; let output = ''; const deflate = zlib.createDeflateRaw({ dictionary: spdyDict }); @@ -167,7 +168,9 @@ function deflateRawResetDictionaryTest() { }); } -basicDictionaryTest(); -deflateResetDictionaryTest(); -rawDictionaryTest(); -deflateRawResetDictionaryTest(); +for (const dict of [spdyDict, spdyDictUint8Array]) { + basicDictionaryTest(dict); + deflateResetDictionaryTest(dict); + rawDictionaryTest(dict); + deflateRawResetDictionaryTest(dict); +} diff --git a/test/parallel/test-zlib-not-string-or-buffer.js b/test/parallel/test-zlib-not-string-or-buffer.js index 3f58583e034ca8..510e111f709699 100644 --- a/test/parallel/test-zlib-not-string-or-buffer.js +++ b/test/parallel/test-zlib-not-string-or-buffer.js @@ -7,7 +7,8 @@ require('../common'); const assert = require('assert'); const zlib = require('zlib'); -const expected = /^TypeError: Not a string or buffer$/; +const expected = + /^TypeError: "buffer" argument must be a string, Buffer, or Uint8Array$/; assert.throws(() => { zlib.deflateSync(undefined); }, expected); assert.throws(() => { zlib.deflateSync(null); }, expected);