Skip to content

Commit

Permalink
zlib: support all ArrayBufferView types
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
TimothyGu committed Apr 12, 2017
1 parent a8f460f commit 2ced07c
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 43 deletions.
69 changes: 56 additions & 13 deletions doc/api/zlib.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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][].

Expand All @@ -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
27 changes: 13 additions & 14 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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');
}
}

Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
}
Expand All @@ -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;

Expand Down Expand Up @@ -417,7 +416,7 @@ class Zlib extends Transform {
self.push(out);
} else {
buffers.push(out);
nread += out.length;
nread += out.byteLength;
}
}

Expand Down
26 changes: 15 additions & 11 deletions test/parallel/test-zlib-convenience-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-zlib-deflate-constructors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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$')
);
3 changes: 1 addition & 2 deletions test/parallel/test-zlib-dictionary.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-zlib-not-string-or-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 2ced07c

Please sign in to comment.