From 24012a879d2fe17287a857ee20f696a78590db15 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Tue, 1 Dec 2015 08:11:18 -0600 Subject: [PATCH] util: make inspect more reliable 34a35919e165cba6d5972e004e6b2cbdf2f4c65a added pretty printing for TypedArray, ArrayBuffer, and DataView. This change allows inspecting those across different contexts. Since instanceof does not work across contexts, we can use v8::Value::IsTypedArray, v8::Value::IsArrayBuffer, and v8::Value::IsDataView PR-URL: https://github.com/nodejs/node/pull/4098 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- lib/util.js | 21 ++-------- src/node_util.cc | 21 ++++++++++ test/parallel/test-util-inspect.js | 62 ++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 17 deletions(-) diff --git a/lib/util.js b/lib/util.js index 6447e463be3..afc8bf280ca 100644 --- a/lib/util.js +++ b/lib/util.js @@ -290,7 +290,7 @@ function formatValue(ctx, value, recurseTimes) { } // Fast path for ArrayBuffer. Can't do the same for DataView because it // has a non-primitive .buffer property that we need to recurse for. - if (value instanceof ArrayBuffer) { + if (binding.isArrayBuffer(value)) { return `${getConstructorOf(value).name}` + ` { byteLength: ${formatNumber(ctx, value.byteLength)} }`; } @@ -328,18 +328,18 @@ function formatValue(ctx, value, recurseTimes) { keys.unshift('size'); empty = value.size === 0; formatter = formatMap; - } else if (value instanceof ArrayBuffer) { + } else if (binding.isArrayBuffer(value)) { braces = ['{', '}']; keys.unshift('byteLength'); visibleKeys.byteLength = true; - } else if (value instanceof DataView) { + } else if (binding.isDataView(value)) { braces = ['{', '}']; // .buffer goes last, it's not a primitive like the others. keys.unshift('byteLength', 'byteOffset', 'buffer'); visibleKeys.byteLength = true; visibleKeys.byteOffset = true; visibleKeys.buffer = true; - } else if (isTypedArray(value)) { + } else if (binding.isTypedArray(value)) { braces = ['[', ']']; formatter = formatTypedArray; if (ctx.showHidden) { @@ -679,19 +679,6 @@ function reduceToSingleString(output, base, braces) { } -function isTypedArray(value) { - return value instanceof Float32Array || - value instanceof Float64Array || - value instanceof Int16Array || - value instanceof Int32Array || - value instanceof Int8Array || - value instanceof Uint16Array || - value instanceof Uint32Array || - value instanceof Uint8Array || - value instanceof Uint8ClampedArray; -} - - // NOTE: These type checking functions intentionally don't use `instanceof` // because it is fragile and can be easily faked with `Object.create()`. exports.isArray = Array.isArray; diff --git a/src/node_util.cc b/src/node_util.cc index a520b8d5f35..dad005d7603 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -30,6 +30,24 @@ static void IsPromise(const FunctionCallbackInfo& args) { } +static void IsTypedArray(const FunctionCallbackInfo& args) { + CHECK_EQ(1, args.Length()); + args.GetReturnValue().Set(args[0]->IsTypedArray()); +} + + +static void IsArrayBuffer(const FunctionCallbackInfo& args) { + CHECK_EQ(1, args.Length()); + args.GetReturnValue().Set(args[0]->IsArrayBuffer()); +} + + +static void IsDataView(const FunctionCallbackInfo& args) { + CHECK_EQ(1, args.Length()); + args.GetReturnValue().Set(args[0]->IsDataView()); +} + + static void GetHiddenValue(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -53,6 +71,9 @@ void Initialize(Local target, env->SetMethod(target, "isMapIterator", IsMapIterator); env->SetMethod(target, "isSetIterator", IsSetIterator); env->SetMethod(target, "isPromise", IsPromise); + env->SetMethod(target, "isTypedArray", IsTypedArray); + env->SetMethod(target, "isArrayBuffer", IsArrayBuffer); + env->SetMethod(target, "isDataView", IsDataView); env->SetMethod(target, "getHiddenValue", GetHiddenValue); } diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 93ac7157fdb..3a033eb3aac 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -2,6 +2,7 @@ var common = require('../common'); var assert = require('assert'); var util = require('util'); +const vm = require('vm'); assert.equal(util.inspect(1), '1'); assert.equal(util.inspect(false), 'false'); @@ -68,6 +69,35 @@ for (const showHidden of [true, false]) { ' y: 1337 }'); } +// Now do the same checks but from a different context +for (const showHidden of [true, false]) { + const ab = vm.runInNewContext('new ArrayBuffer(4)'); + const dv = vm.runInNewContext('new DataView(ab, 1, 2)', { ab: ab }); + assert.equal(util.inspect(ab, showHidden), 'ArrayBuffer { byteLength: 4 }'); + assert.equal(util.inspect(new DataView(ab, 1, 2), showHidden), + 'DataView {\n' + + ' byteLength: 2,\n' + + ' byteOffset: 1,\n' + + ' buffer: ArrayBuffer { byteLength: 4 } }'); + assert.equal(util.inspect(ab, showHidden), 'ArrayBuffer { byteLength: 4 }'); + assert.equal(util.inspect(dv, showHidden), + 'DataView {\n' + + ' byteLength: 2,\n' + + ' byteOffset: 1,\n' + + ' buffer: ArrayBuffer { byteLength: 4 } }'); + ab.x = 42; + dv.y = 1337; + assert.equal(util.inspect(ab, showHidden), + 'ArrayBuffer { byteLength: 4, x: 42 }'); + assert.equal(util.inspect(dv, showHidden), + 'DataView {\n' + + ' byteLength: 2,\n' + + ' byteOffset: 1,\n' + + ' buffer: ArrayBuffer { byteLength: 4, x: 42 },\n' + + ' y: 1337 }'); +} + + [ Float32Array, Float64Array, Int16Array, @@ -94,6 +124,38 @@ for (const showHidden of [true, false]) { assert.equal(util.inspect(array, false), `${constructor.name} [ 65, 97 ]`); }); +// Now check that declaring a TypedArray in a different context works the same +[ Float32Array, + Float64Array, + Int16Array, + Int32Array, + Int8Array, + Uint16Array, + Uint32Array, + Uint8Array, + Uint8ClampedArray ].forEach(constructor => { + const length = 2; + const byteLength = length * constructor.BYTES_PER_ELEMENT; + const array = vm.runInNewContext('new constructor(new ArrayBuffer(' + + 'byteLength), 0, length)', + { constructor: constructor, + byteLength: byteLength, + length: length + }); + array[0] = 65; + array[1] = 97; + assert.equal(util.inspect(array, true), + `${constructor.name} [\n` + + ` 65,\n` + + ` 97,\n` + + ` [BYTES_PER_ELEMENT]: ${constructor.BYTES_PER_ELEMENT},\n` + + ` [length]: ${length},\n` + + ` [byteLength]: ${byteLength},\n` + + ` [byteOffset]: 0,\n` + + ` [buffer]: ArrayBuffer { byteLength: ${byteLength} } ]`); + assert.equal(util.inspect(array, false), `${constructor.name} [ 65, 97 ]`); + }); + // Due to the hash seed randomization it's not deterministic the order that // the following ways this hash is displayed. // See http://codereview.chromium.org/9124004/