util: make inspect more reliable

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: #4098 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
nodejs · Dec 2, 2015 · 24012a8 · 24012a8
1 parent 451254e
commit 24012a8
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 17 deletions.
diff --git 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
@@ -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);
 
@@ -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);
 }
 

diff --git 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/