Skip to content

Commit

Permalink
util: make inspect more reliable
Browse files Browse the repository at this point in the history 
34a3591 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: nodejs#4098
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
@evanlucas
evanlucas authored and Michael Scovetta committed Apr 2, 2016
1 parent 8a3b610 commit 7e0bc7e
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 17 deletions.
21 changes: 4 additions & 17 deletions lib/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)} }`;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
21 changes: 21 additions & 0 deletions src/node_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,24 @@ static void IsPromise(const FunctionCallbackInfo<Value>& args) {
}


static void IsTypedArray(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(1, args.Length());
args.GetReturnValue().Set(args[0]->IsTypedArray());
}


static void IsArrayBuffer(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(1, args.Length());
args.GetReturnValue().Set(args[0]->IsArrayBuffer());
}


static void IsDataView(const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(1, args.Length());
args.GetReturnValue().Set(args[0]->IsDataView());
}


static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand All @@ -53,6 +71,9 @@ void Initialize(Local<Object> 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);
}

Expand Down
62 changes: 62 additions & 0 deletions test/parallel/test-util-inspect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
Expand All @@ -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/
Expand Down

0 comments on commit 7e0bc7e

Please sign in to comment.