From 2ced07ccaf7682b9ec8fb3bcc3dc8d2bb2798c61 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 3 Apr 2017 18:15:13 -0700 Subject: [PATCH] zlib: support all ArrayBufferView types PR-URL: https://github.com/nodejs/node/pull/12223 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- doc/api/zlib.md | 69 +++++++++++++++---- lib/zlib.js | 27 ++++---- .../parallel/test-zlib-convenience-methods.js | 26 ++++--- .../test-zlib-deflate-constructors.js | 3 +- test/parallel/test-zlib-dictionary.js | 3 +- .../test-zlib-not-string-or-buffer.js | 4 +- 6 files changed, 89 insertions(+), 43 deletions(-) diff --git a/doc/api/zlib.md b/doc/api/zlib.md index 928070d5778ca7..a91456bb0b835e 100644 --- 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][]. -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. -- `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][]. -- `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][]. -- `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][]. -- `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][]. -- `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][]. -- `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][]. -- `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 index 07040c3ebc581e..00be56dffc44fb 100644 --- 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 index df56f21ff49529..78bb105906fb00 100644 --- 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 index c495d2d11d5acb..49695fbaa55585 100644 --- 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 index a3b55bc72d4fc6..1662e63bca135c 100644 --- 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 index 510e111f709699..43488f00754342 100644 --- 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);