zlib: support all ArrayBufferView types

PR-URL: #12223 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs · Apr 12, 2017 · 2ced07c · 2ced07c
1 parent a8f460f
commit 2ced07c
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 43 deletions.
diff --git a/doc/api/zlib.md b/doc/api/zlib.md
@@ -300,7 +300,7 @@ ignored by the decompression classes.
 * `level` {integer} (compression only)
 * `memLevel` {integer} (compression only)
 * `strategy` {integer} (compression only)
-* `dictionary` {Buffer|Uint8Array} (deflate/inflate only, empty dictionary by
+* `dictionary` {Buffer|TypedArray|DataView} (deflate/inflate only, empty dictionary by
   default)
 
 See the description of `deflateInit2` and `inflateInit2` at
@@ -477,9 +477,9 @@ Returns a new [Unzip][] object with an [options][].
 
 <!--type=misc-->
 
-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)`.
+All of these take a [`Buffer`][], [`TypedArray`][], [`DataView`][], 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.
@@ -488,6 +488,9 @@ without a callback.
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -496,19 +499,25 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Compress a chunk of data with [Deflate][].
 
 ### zlib.deflateRaw(buffer[, options], callback)
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -517,19 +526,25 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Compress a chunk of data with [DeflateRaw][].
 
 ### zlib.gunzip(buffer[, options], callback)
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -538,19 +553,25 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Decompress a chunk of data with [Gunzip][].
 
 ### zlib.gzip(buffer[, options], callback)
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -559,19 +580,25 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Compress a chunk of data with [Gzip][].
 
 ### zlib.inflate(buffer[, options], callback)
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -580,19 +607,25 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Decompress a chunk of data with [Inflate][].
 
 ### zlib.inflateRaw(buffer[, options], callback)
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -601,19 +634,25 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Decompress a chunk of data with [InflateRaw][].
 
 ### zlib.unzip(buffer[, options], callback)
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
@@ -622,12 +661,15 @@ changes:
 <!-- YAML
 added: v0.11.12
 changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: The `buffer` parameter can be any TypedArray or DataView now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/12001
     description: The `buffer` parameter can be an Uint8Array now.
 -->
 
-- `buffer` {Buffer|Uint8Array|string}
+- `buffer` {Buffer|TypedArray|DataView|string}
 
 Decompress a chunk of data with [Unzip][].
 
@@ -644,5 +686,6 @@ Decompress a chunk of data with [Unzip][].
 [InflateRaw]: #zlib_class_zlib_inflateraw
 [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
+[`Buffer`]: buffer.html#buffer_class_buffer
+[`DataView`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView
+[`TypedArray`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray
diff --git a/lib/zlib.js b/lib/zlib.js
@@ -24,7 +24,6 @@
 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;
@@ -79,9 +78,9 @@ function isInvalidStrategy(strategy) {
 }
 
 function zlibBuffer(engine, buffer, callback) {
-  // Streams do not support non-Buffer Uint8Arrays yet. Convert it to a
+  // Streams do not support non-Buffer ArrayBufferViews yet. Convert it to a
   // Buffer without copying.
-  if (isUint8Array(buffer) &&
+  if (ArrayBuffer.isView(buffer) &&
       Object.getPrototypeOf(buffer) !== Buffer.prototype) {
     buffer = Buffer.from(buffer.buffer, buffer.byteOffset, buffer.byteLength);
   }
@@ -99,7 +98,7 @@ function zlibBuffer(engine, buffer, callback) {
     var chunk;
     while (null !== (chunk = engine.read())) {
       buffers.push(chunk);
-      nread += chunk.length;
+      nread += chunk.byteLength;
     }
     engine.once('readable', flow);
   }
@@ -129,9 +128,9 @@ function zlibBuffer(engine, buffer, callback) {
 function zlibBufferSync(engine, buffer) {
   if (typeof buffer === 'string')
     buffer = Buffer.from(buffer);
-  else if (!isUint8Array(buffer))
-    throw new TypeError('"buffer" argument must be a string, Buffer, or ' +
-                        'Uint8Array');
+  else if (!ArrayBuffer.isView(buffer))
+    throw new TypeError('"buffer" argument must be a string, Buffer, ' +
+                        'TypedArray, or DataView');
 
   var flushFlag = engine._finishFlushFlag;
 
@@ -214,9 +213,9 @@ class Zlib extends Transform {
       throw new TypeError('Invalid strategy: ' + opts.strategy);
 
     if (opts.dictionary) {
-      if (!isUint8Array(opts.dictionary)) {
+      if (!ArrayBuffer.isView(opts.dictionary)) {
         throw new TypeError(
-          'Invalid dictionary: it should be a Buffer or an Uint8Array');
+          'Invalid dictionary: it should be a Buffer, TypedArray, or DataView');
       }
     }
 
@@ -309,9 +308,9 @@ class Zlib extends Transform {
     var flushFlag;
     var ws = this._writableState;
     var ending = ws.ending || ws.ended;
-    var last = ending && (!chunk || ws.length === chunk.length);
+    var last = ending && (!chunk || ws.length === chunk.byteLength);
 
-    if (chunk !== null && !isUint8Array(chunk))
+    if (chunk !== null && !ArrayBuffer.isView(chunk))
       return cb(new TypeError('invalid input'));
 
     if (!this._handle)
@@ -328,7 +327,7 @@ class Zlib extends Transform {
       flushFlag = this._flushFlag;
       // once we've flushed the last of the queue, stop flushing and
       // go back to the normal behavior.
-      if (chunk.length >= ws.length) {
+      if (chunk.byteLength >= ws.length) {
         this._flushFlag = this._opts.flush || constants.Z_NO_FLUSH;
       }
     }
@@ -337,7 +336,7 @@ class Zlib extends Transform {
   }
 
   _processChunk(chunk, flushFlag, cb) {
-    var availInBefore = chunk && chunk.length;
+    var availInBefore = chunk && chunk.byteLength;
     var availOutBefore = this._chunkSize - this._offset;
     var inOff = 0;
 
@@ -417,7 +416,7 @@ class Zlib extends Transform {
           self.push(out);
         } else {
           buffers.push(out);
-          nread += out.length;
+          nread += out.byteLength;
         }
       }
 

diff --git a/test/parallel/test-zlib-convenience-methods.js b/test/parallel/test-zlib-convenience-methods.js
@@ -26,24 +26,28 @@ const common = require('../common');
 const assert = require('assert');
 const zlib = require('zlib');
 
-const expectStr = 'blahblahblahblahblahblah';
+// Must be a multiple of 4 characters in total to test all ArrayBufferView
+// types.
+const expectStr = 'blah'.repeat(8);
 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'],
+for (const [type, expect] of [
+  ['string', expectStr],
+  ['Buffer', expectBuf],
+  ...common.getArrayBufferViews(expectBuf).map((obj) =>
+    [obj[Symbol.toStringTag], obj]
+  )
 ]) {
-  for (const [type, expect] of [
-    ['string', expectStr],
-    ['Buffer', expectBuf],
-    ['Uint8Array', expectUint8Array]
+  for (const method of [
+    ['gzip', 'gunzip'],
+    ['gzip', 'unzip'],
+    ['deflate', 'inflate'],
+    ['deflateRaw', 'inflateRaw'],
   ]) {
     zlib[method[0]](expect, opts, common.mustCall((err, result) => {
       zlib[method[1]](result, opts, common.mustCall((err, result) => {

diff --git a/test/parallel/test-zlib-deflate-constructors.js b/test/parallel/test-zlib-deflate-constructors.js
@@ -107,5 +107,6 @@ 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 or an Uint8Array$/
+  new RegExp('^TypeError: Invalid dictionary: it should be a Buffer, ' +
+             'TypedArray, or DataView$')
 );
diff --git a/test/parallel/test-zlib-dictionary.js b/test/parallel/test-zlib-dictionary.js
@@ -41,7 +41,6 @@ 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',
@@ -168,7 +167,7 @@ function deflateRawResetDictionaryTest(spdyDict) {
   });
 }
 
-for (const dict of [spdyDict, spdyDictUint8Array]) {
+for (const dict of [spdyDict, ...common.getArrayBufferViews(spdyDict)]) {
   basicDictionaryTest(dict);
   deflateResetDictionaryTest(dict);
   rawDictionaryTest(dict);

diff --git a/test/parallel/test-zlib-not-string-or-buffer.js b/test/parallel/test-zlib-not-string-or-buffer.js
@@ -7,8 +7,8 @@ require('../common');
 const assert = require('assert');
 const zlib = require('zlib');
 
-const expected =
-    /^TypeError: "buffer" argument must be a string, Buffer, or Uint8Array$/;
+const expected = new RegExp('^TypeError: "buffer" argument must be a string, ' +
+                            'Buffer, TypedArray, or DataView$');
 
 assert.throws(() => { zlib.deflateSync(undefined); }, expected);
 assert.throws(() => { zlib.deflateSync(null); }, expected);