From c93e2678f0c896c9d7f85b2174ec307601373fb8 Mon Sep 17 00:00:00 2001
From: Brian White <mscdex@mscdex.net>
Date: Thu, 15 Oct 2015 15:55:42 -0400
Subject: [PATCH] util: fix constructor/instanceof checks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

These new checks are similar to the one introduced in 089d68861,
but for other types of objects. Specifically, if an object was
created in a different context, the constructor object will not be
the same as the constructor object in the current context, so we
have to compare constructor names instead.

PR-URL: https://github.com/nodejs/node/pull/3385
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
---
 lib/util.js                        | 20 ++++++++++++--------
 test/parallel/test-util-inspect.js | 10 ++++++++++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/util.js b/lib/util.js
index 0ccfca0304af9c..c07edfe87d6405 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -198,6 +198,7 @@ function ensureDebugIsInitialized() {
 
 function inspectPromise(p) {
   ensureDebugIsInitialized();
+  // Only create a mirror if the object is a Promise.
   if (!binding.isPromise(p))
     return null;
   const mirror = Debug.MakeMirror(p, true);
@@ -292,16 +293,19 @@ function formatValue(ctx, value, recurseTimes) {
   var constructor = getConstructorOf(value);
   var base = '', empty = false, braces, formatter;
 
+  // We can't compare constructors for various objects using a comparison like
+  // `constructor === Array` because the object could have come from a different
+  // context and thus the constructor won't match. Instead we check the
+  // constructor names (including those up the prototype chain where needed) to
+  // determine object types.
   if (Array.isArray(value)) {
-    // We can't use `constructor === Array` because this could
-    // have come from a Debug context.
-    // Otherwise, an Array will print "Array [...]".
+    // Unset the constructor to prevent "Array [...]" for ordinary arrays.
     if (constructor && constructor.name === 'Array')
       constructor = null;
     braces = ['[', ']'];
     empty = value.length === 0;
     formatter = formatArray;
-  } else if (value instanceof Set) {
+  } else if (objectToString(value) === '[object Set]') {
     braces = ['{', '}'];
     // With `showHidden`, `length` will display as a hidden property for
     // arrays. For consistency's sake, do the same for `size`, even though this
@@ -310,7 +314,7 @@ function formatValue(ctx, value, recurseTimes) {
       keys.unshift('size');
     empty = value.size === 0;
     formatter = formatSet;
-  } else if (value instanceof Map) {
+  } else if (objectToString(value) === '[object Map]') {
     braces = ['{', '}'];
     // Ditto.
     if (ctx.showHidden)
@@ -318,8 +322,7 @@ function formatValue(ctx, value, recurseTimes) {
     empty = value.size === 0;
     formatter = formatMap;
   } else {
-    // Only create a mirror if the object superficially looks like a Promise.
-    var promiseInternals = value instanceof Promise && inspectPromise(value);
+    var promiseInternals = inspectPromise(value);
     if (promiseInternals) {
       braces = ['{', '}'];
       formatter = formatPromise;
@@ -335,7 +338,8 @@ function formatValue(ctx, value, recurseTimes) {
         empty = false;
         formatter = formatCollectionIterator;
       } else {
-        if (constructor === Object)
+        // Unset the constructor to prevent "Object {...}" for ordinary objects.
+        if (constructor && constructor.name === 'Object')
           constructor = null;
         braces = ['{', '}'];
         empty = true;  // No other data than keys.
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index 320d5e444ad80a..5831c40e77e930 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -141,6 +141,16 @@ for (const o of vals) {
 
 assert.strictEqual(util.inspect(valsOutput), '[ [ 1, 2 ] ]');
 
+// test for other constructors in different context
+var obj = require('vm').runInNewContext('(function(){return {}})()', {});
+assert.strictEqual(util.inspect(obj), '{}');
+obj = require('vm').runInNewContext('var m=new Map();m.set(1,2);m', {});
+assert.strictEqual(util.inspect(obj), 'Map { 1 => 2 }');
+obj = require('vm').runInNewContext('var s=new Set();s.add(1);s.add(2);s', {});
+assert.strictEqual(util.inspect(obj), 'Set { 1, 2 }');
+obj = require('vm').runInNewContext('fn=function(){};new Promise(fn,fn)', {});
+assert.strictEqual(util.inspect(obj), 'Promise { <pending> }');
+
 // test for property descriptors
 var getter = Object.create(null, {
   a: {