From 4fceb19c0b895d0f00149e71d1485daf9ad198c9 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 20 Mar 2017 23:56:19 -0700 Subject: [PATCH 1/3] benchmark: add Buffer.isBuffer --- benchmark/buffers/buffer-isbuffer.js | 39 ++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 benchmark/buffers/buffer-isbuffer.js diff --git a/benchmark/buffers/buffer-isbuffer.js b/benchmark/buffers/buffer-isbuffer.js new file mode 100644 index 00000000000000..7c8387219cbec3 --- /dev/null +++ b/benchmark/buffers/buffer-isbuffer.js @@ -0,0 +1,39 @@ +'use strict'; +const common = require('../common'); +const { Buffer } = require('buffer'); + +function FakeBuffer() {} +FakeBuffer.prototype = Object.create(Buffer.prototype); + +// Refs: https://github.com/nodejs/node/issues/9531#issuecomment-265611061 +class ExtensibleBuffer extends Buffer { + constructor() { + super(new ArrayBuffer(0), 0, 0); + Object.setPrototypeOf(this, new.target.prototype); + } +} + +const inputs = { + primitive: false, + object: {}, + fake: new FakeBuffer(), + subclassed: new ExtensibleBuffer(), + fastbuffer: Buffer.alloc(1), + slowbuffer: Buffer.allocUnsafeSlow(0), + uint8array: new Uint8Array(1) +}; + +const bench = common.createBenchmark(main, { + type: Object.keys(inputs), + n: [1e7] +}); + +function main(conf) { + const n = conf.n | 0; + const input = inputs[conf.type]; + + bench.start(); + for (var i = 0; i < n; i++) + Buffer.isBuffer(input); + bench.end(n); +} From 2eab4f817212c97ac608a76871fe3954bdfed7e8 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 20 Mar 2017 23:51:48 -0700 Subject: [PATCH 2/3] lib: use Buffer.isBuffer() consistently --- lib/_http_outgoing.js | 4 ++-- lib/buffer.js | 4 ++-- lib/fs.js | 2 +- lib/net.js | 8 ++++---- lib/readline.js | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index aeaa85eec81494..e93bcb19eec980 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -645,7 +645,7 @@ function write_(msg, chunk, encoding, callback, fromEnd) { return true; } - if (!fromEnd && typeof chunk !== 'string' && !(chunk instanceof Buffer)) { + if (!fromEnd && typeof chunk !== 'string' && !Buffer.isBuffer(chunk)) { throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument', ['string', 'buffer']); } @@ -743,7 +743,7 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) { var uncork; if (chunk) { - if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) { + if (typeof chunk !== 'string' && !Buffer.isBuffer(chunk)) { throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'first argument', ['string', 'buffer']); } diff --git a/lib/buffer.js b/lib/buffer.js index 7a44ca0f260539..fff7485cb004f6 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -567,7 +567,7 @@ Buffer.byteLength = byteLength; Object.defineProperty(Buffer.prototype, 'parent', { enumerable: true, get() { - if (!(this instanceof Buffer)) + if (!Buffer.isBuffer(this)) return undefined; return this.buffer; } @@ -575,7 +575,7 @@ Object.defineProperty(Buffer.prototype, 'parent', { Object.defineProperty(Buffer.prototype, 'offset', { enumerable: true, get() { - if (!(this instanceof Buffer)) + if (!Buffer.isBuffer(this)) return undefined; return this.byteOffset; } diff --git a/lib/fs.js b/lib/fs.js index 24af0e58a3dc99..3c90cb8f94c2d2 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2198,7 +2198,7 @@ WriteStream.prototype.open = function() { WriteStream.prototype._write = function(data, encoding, cb) { - if (!(data instanceof Buffer)) + if (!Buffer.isBuffer(data)) return this.emit('error', new Error('Invalid data')); if (typeof this.fd !== 'number') { diff --git a/lib/net.js b/lib/net.js index d693b25d46f54b..b3d0b1226f63e7 100644 --- a/lib/net.js +++ b/lib/net.js @@ -694,7 +694,7 @@ protoGetter('localPort', function localPort() { Socket.prototype.write = function(chunk, encoding, cb) { - if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) { + if (typeof chunk !== 'string' && !Buffer.isBuffer(chunk)) { throw new TypeError( 'Invalid data, chunk must be a string or buffer, not ' + typeof chunk); } @@ -752,7 +752,7 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { if (err === 0) req._chunks = chunks; } else { var enc; - if (data instanceof Buffer) { + if (Buffer.isBuffer(data)) { enc = 'buffer'; } else { enc = encoding; @@ -821,7 +821,7 @@ protoGetter('bytesWritten', function bytesWritten() { return undefined; state.getBuffer().forEach(function(el) { - if (el.chunk instanceof Buffer) + if (Buffer.isBuffer(el.chunk)) bytes += el.chunk.length; else bytes += Buffer.byteLength(el.chunk, el.encoding); @@ -832,7 +832,7 @@ protoGetter('bytesWritten', function bytesWritten() { for (var i = 0; i < data.length; i++) { const chunk = data[i]; - if (data.allBuffers || chunk instanceof Buffer) + if (data.allBuffers || Buffer.isBuffer(chunk)) bytes += chunk.length; else bytes += Buffer.byteLength(chunk.chunk, chunk.encoding); diff --git a/lib/readline.js b/lib/readline.js index 5e96a04b1c5e98..f4d7b273777087 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -973,7 +973,7 @@ Interface.prototype._ttyWrite = function(s, key) { // falls through default: - if (s instanceof Buffer) + if (Buffer.isBuffer(s)) s = s.toString('utf-8'); if (s) { From ee2d44a7050695b14b7b9a44778e4eb59f6e8a1d Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 20 Mar 2017 23:56:04 -0700 Subject: [PATCH 3/3] buffer: stricter Buffer.isBuffer Make "fake" Buffer subclasses whose instances are not valid Uint8Arrays fail the test. Fixes: https://github.com/nodejs/node/issues/11954 --- lib/buffer.js | 9 +++++++- test/parallel/test-buffer-isbuffer.js | 31 +++++++++++++++++++++++++++ test/parallel/test-util.js | 1 + 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-buffer-isbuffer.js diff --git a/lib/buffer.js b/lib/buffer.js index fff7485cb004f6..ae690bb9874db6 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -420,7 +420,14 @@ function fromObject(obj) { // Static methods Buffer.isBuffer = function isBuffer(b) { - return b instanceof Buffer; + if (!isUint8Array(b)) + return false; + // Subclassing Buffer is not officially supported currently, and actually + // subclassing it requires some awkward code that users are unlikely to do. + // Therefore prioritize the fast case over the full `instanceof`. + // Refs: https://github.com/nodejs/node/issues/9531#issuecomment-265611061 + const proto = Object.getPrototypeOf(b); + return proto === Buffer.prototype || proto instanceof Buffer.prototype; }; diff --git a/test/parallel/test-buffer-isbuffer.js b/test/parallel/test-buffer-isbuffer.js new file mode 100644 index 00000000000000..246fb0817661b0 --- /dev/null +++ b/test/parallel/test-buffer-isbuffer.js @@ -0,0 +1,31 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const { Buffer } = require('buffer'); + +const t = (val, exp) => assert.strictEqual(Buffer.isBuffer(val), exp); + +function FakeBuffer() {} +FakeBuffer.prototype = Object.create(Buffer.prototype); + +class ExtensibleBuffer extends Buffer { + constructor() { + super(new ArrayBuffer(0), 0, 0); + Object.setPrototypeOf(this, new.target.prototype); + } +} + +t(0, false); +t(true, false); +t('foo', false); +t(Symbol(), false); +t(null, false); +t(undefined, false); +t(() => {}, false); + +t({}, false); +t(new Uint8Array(2), false); +t(new FakeBuffer(), false); +t(new ExtensibleBuffer(), true); +t(Buffer.from('foo'), true); +t(Buffer.allocUnsafeSlow(2), true); diff --git a/test/parallel/test-util.js b/test/parallel/test-util.js index 3b2729c107b4b1..1f9a6ef34e8676 100644 --- a/test/parallel/test-util.js +++ b/test/parallel/test-util.js @@ -96,6 +96,7 @@ assert.strictEqual(true, util.isPrimitive(NaN)); assert.strictEqual(true, util.isPrimitive(Symbol('symbol'))); // isBuffer +assert.strictEqual(util.isBuffer, Buffer.isBuffer); assert.strictEqual(false, util.isBuffer('foo')); assert.strictEqual(true, util.isBuffer(Buffer.from('foo')));