zlib: support all ArrayBufferView types

PR-URL: https://github.com/nodejs/node/pull/12223 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
2017-04-03 18:15:13 -07:00 · 2017-04-03 18:15:13 -07:00 · 2ced07ccaf
commit 2ced07ccaf
parent a8f460f12d
6 changed files with 89 additions and 43 deletions
--- 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,12 +499,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}

 Compress a chunk of data with [Deflate][].

@ -509,6 +515,9 @@ Compress a chunk of data with [Deflate][].
 <!-- 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,12 +526,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}

 Compress a chunk of data with [DeflateRaw][].

@ -530,6 +542,9 @@ Compress a chunk of data with [DeflateRaw][].
 <!-- 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,12 +553,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 [Gunzip][].

@ -551,6 +569,9 @@ Decompress a chunk of data with [Gunzip][].
 <!-- 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,12 +580,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}

 Compress a chunk of data with [Gzip][].

@ -572,6 +596,9 @@ Compress a chunk of data with [Gzip][].
 <!-- 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,12 +607,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 [Inflate][].

@ -593,6 +623,9 @@ Decompress a chunk of data with [Inflate][].
 <!-- 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,12 +634,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 [InflateRaw][].

@ -614,6 +650,9 @@ Decompress a chunk of data with [InflateRaw][].
 <!-- 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
--- 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;
        }
      }

--- 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) => {
--- 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$')
 );
--- 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);
--- 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);