zlib: support Uint8Array in convenience methods

Also support Uint8Array as a `dictionary` option. PR-URL: #12001 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
nodejs · Apr 3, 2017 · 91383e4 · 91383e4
1 parent 2d039ff
commit 91383e4
Show file tree

Hide file tree

Showing 6 changed files with 176 additions and 87 deletions.
diff --git a/doc/api/zlib.md b/doc/api/zlib.md
@@ -278,6 +278,9 @@ Compression strategy.
 <!-- YAML
 added: v0.11.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `dictionary` option can be an Uint8Array now.
   - version: v5.11.0
     pr-url: https://github.com/nodejs/node/pull/6069
     description: The `finishFlush` option is supported now.
@@ -290,14 +293,15 @@ Each class takes an `options` object.  All options are optional.
 Note that some options are only relevant when compressing, and are
 ignored by the decompression classes.
 
-* `flush` (default: `zlib.constants.Z_NO_FLUSH`)
-* `finishFlush` (default: `zlib.constants.Z_FINISH`)
-* `chunkSize` (default: 16*1024)
-* `windowBits`
-* `level` (compression only)
-* `memLevel` (compression only)
-* `strategy` (compression only)
-* `dictionary` (deflate/inflate only, empty dictionary by default)
+* `flush` {integer} (default: `zlib.constants.Z_NO_FLUSH`)
+* `finishFlush` {integer} (default: `zlib.constants.Z_FINISH`)
+* `chunkSize` {integer} (default: 16\*1024)
+* `windowBits` {integer}
+* `level` {integer} (compression only)
+* `memLevel` {integer} (compression only)
+* `strategy` {integer} (compression only)
+* `dictionary` {Buffer|Uint8Array} (deflate/inflate only, empty dictionary by
+  default)
 
 See the description of `deflateInit2` and `inflateInit2` at
 <http://zlib.net/manual.html#Advanced> for more information on these.
@@ -473,89 +477,159 @@ Returns a new [Unzip][] object with an [options][].
 
 <!--type=misc-->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.deflateSync(buf[, options])
+### zlib.deflateSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.deflateRawSync(buf[, options])
+### zlib.deflateRawSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.gunzipSync(buf[, options])
+### zlib.gunzipSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.gzipSync(buf[, options])
+### zlib.gzipSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.inflateSync(buf[, options])
+### zlib.inflateSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.inflateRawSync(buf[, options])
+### zlib.inflateRawSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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)
 <!-- YAML
 added: v0.6.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
-### zlib.unzipSync(buf[, options])
+### zlib.unzipSync(buffer[, options])
 <!-- YAML
 added: v0.11.12
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12001
+    description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-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
@@ -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
@@ -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
@@ -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$/
 );