Skip to content

Commit

Permalink
util: fix constructor/instanceof checks
Browse files Browse the repository at this point in the history
These new checks are similar to the one introduced in 089d688,
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: nodejs#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>
  • Loading branch information
mscdex authored and Michael Scovetta committed Apr 2, 2016
1 parent 1a3dbd3 commit a972159
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
20 changes: 12 additions & 8 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -299,16 +300,19 @@ function formatValue(ctx, value, recurseTimes) {
var base = '', empty = false, braces;
var formatter = formatObject;

// 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
Expand All @@ -317,7 +321,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)
Expand Down Expand Up @@ -347,8 +351,7 @@ function formatValue(ctx, value, recurseTimes) {
'buffer');
}
} 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;
Expand All @@ -364,7 +367,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.
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,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: {
Expand Down

0 comments on commit a972159

Please sign in to comment.