-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow all ArrayBufferView types as Buffers #12223
Changes from 4 commits
a8abe9c
dae94ff
c58b945
07a399b
1459f27
039208e
072e898
1315240
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ | |
const Buffer = require('buffer').Buffer; | ||
const internalUtil = require('internal/util'); | ||
const Transform = require('_stream_transform'); | ||
const { isUint8Array } = process.binding('util'); | ||
const { isArrayBufferView } = process.binding('util'); | ||
const binding = process.binding('zlib'); | ||
const assert = require('assert').ok; | ||
const kMaxLength = require('buffer').kMaxLength; | ||
|
@@ -79,9 +79,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 (isArrayBufferView(buffer) && | ||
Object.getPrototypeOf(buffer) !== Buffer.prototype) { | ||
buffer = Buffer.from(buffer.buffer, buffer.byteOffset, buffer.byteLength); | ||
} | ||
|
@@ -99,7 +99,7 @@ function zlibBuffer(engine, buffer, callback) { | |
var chunk; | ||
while (null !== (chunk = engine.read())) { | ||
buffers.push(chunk); | ||
nread += chunk.length; | ||
nread += chunk.byteLength; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so, but I'd rather err on the side of replacing too many |
||
} | ||
engine.once('readable', flow); | ||
} | ||
|
@@ -129,9 +129,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 (!isArrayBufferView(buffer)) | ||
throw new TypeError('"buffer" argument must be a string, Buffer, ' + | ||
'TypedArray, or DataView'); | ||
|
||
var flushFlag = engine._finishFlushFlag; | ||
|
||
|
@@ -214,9 +214,9 @@ class Zlib extends Transform { | |
throw new TypeError('Invalid strategy: ' + opts.strategy); | ||
|
||
if (opts.dictionary) { | ||
if (!isUint8Array(opts.dictionary)) { | ||
if (!isArrayBufferView(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 +309,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 && !isArrayBufferView(chunk)) | ||
return cb(new TypeError('invalid input')); | ||
|
||
if (!this._handle) | ||
|
@@ -328,7 +328,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 +337,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 +417,7 @@ class Zlib extends Transform { | |
self.push(out); | ||
} else { | ||
buffers.push(out); | ||
nread += out.length; | ||
nread += out.byteLength; | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this equivalent to
ArrayBuffer.isView()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lpinca yep, those do the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think the standard version should be better in this case.