Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: Handle null prototype on inspect #22331

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 64 additions & 23 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ function getEmptyFormatArray() {
}

function getConstructorName(obj) {
let firstProto;
while (obj) {
const descriptor = Object.getOwnPropertyDescriptor(obj, 'constructor');
if (descriptor !== undefined &&
Expand All @@ -335,25 +336,35 @@ function getConstructorName(obj) {
}

obj = Object.getPrototypeOf(obj);
if (firstProto === undefined) {
firstProto = obj;
}
}

if (firstProto === null) {
return null;
}
// TODO(BridgeAR): Improve prototype inspection.
// We could use inspect on the prototype itself to improve the output.

return '';
}

function getPrefix(constructor, tag, fallback) {
if (constructor === null) {
if (tag !== '') {
return `[${fallback}: null prototype] [${tag}] `;
}
return `[${fallback}: null prototype] `;
}

if (constructor !== '') {
if (tag !== '' && constructor !== tag) {
return `${constructor} [${tag}] `;
}
return `${constructor} `;
}

if (tag !== '')
return `[${tag}] `;

if (fallback !== undefined)
return `${fallback} `;

return '';
}

Expand Down Expand Up @@ -427,21 +438,49 @@ function findTypedConstructor(value) {
}
}

let lazyNullPrototypeCache;
// Creates a subclass and name
// the constructor as `${clazz} : null prototype`
function clazzWithNullPrototype(clazz, name) {
if (lazyNullPrototypeCache === undefined) {
lazyNullPrototypeCache = new Map();
} else {
const cachedClass = lazyNullPrototypeCache.get(clazz);
if (cachedClass !== undefined) {
return cachedClass;
}
}
class NullPrototype extends clazz {
get [Symbol.toStringTag]() {
return '';
}
}
Object.defineProperty(NullPrototype.prototype.constructor, 'name',
{ value: `[${name}: null prototype]` });
lazyNullPrototypeCache.set(clazz, NullPrototype);
return NullPrototype;
}

function noPrototypeIterator(ctx, value, recurseTimes) {
let newVal;
// TODO: Create a Subclass in case there's no prototype and show
// `null-prototype`.
if (isSet(value)) {
const clazz = Object.getPrototypeOf(value) || Set;
const clazz = Object.getPrototypeOf(value) ||
clazzWithNullPrototype(Set, 'Set');
newVal = new clazz(setValues(value));
} else if (isMap(value)) {
const clazz = Object.getPrototypeOf(value) || Map;
const clazz = Object.getPrototypeOf(value) ||
clazzWithNullPrototype(Map, 'Map');
newVal = new clazz(mapEntries(value));
} else if (Array.isArray(value)) {
const clazz = Object.getPrototypeOf(value) || Array;
const clazz = Object.getPrototypeOf(value) ||
clazzWithNullPrototype(Array, 'Array');
newVal = new clazz(value.length || 0);
} else if (isTypedArray(value)) {
const clazz = findTypedConstructor(value) || Uint8Array;
let clazz = Object.getPrototypeOf(value);
if (!clazz) {
const constructor = findTypedConstructor(value);
clazz = clazzWithNullPrototype(constructor, constructor.name);
}
newVal = new clazz(value);
}
if (newVal) {
Expand Down Expand Up @@ -527,29 +566,32 @@ function formatRaw(ctx, value, recurseTimes) {
if (Array.isArray(value)) {
keys = getOwnNonIndexProperties(value, filter);
// Only set the constructor for non ordinary ("Array [...]") arrays.
const prefix = getPrefix(constructor, tag);
const prefix = getPrefix(constructor, tag, 'Array');
braces = [`${prefix === 'Array ' ? '' : prefix}[`, ']'];
if (value.length === 0 && keys.length === 0)
return `${braces[0]}]`;
extrasType = kArrayExtrasType;
formatter = formatArray;
} else if (isSet(value)) {
keys = getKeys(value, ctx.showHidden);
const prefix = getPrefix(constructor, tag);
const prefix = getPrefix(constructor, tag, 'Set');
if (value.size === 0 && keys.length === 0)
return `${prefix}{}`;
braces = [`${prefix}{`, '}'];
formatter = formatSet;
} else if (isMap(value)) {
keys = getKeys(value, ctx.showHidden);
const prefix = getPrefix(constructor, tag);
const prefix = getPrefix(constructor, tag, 'Map');
if (value.size === 0 && keys.length === 0)
return `${prefix}{}`;
braces = [`${prefix}{`, '}'];
formatter = formatMap;
} else if (isTypedArray(value)) {
keys = getOwnNonIndexProperties(value, filter);
braces = [`${getPrefix(constructor, tag)}[`, ']'];
const prefix = constructor !== null ?
getPrefix(constructor, tag) :
getPrefix(constructor, tag, findTypedConstructor(value).name);
braces = [`${prefix}[`, ']'];
if (value.length === 0 && keys.length === 0 && !ctx.showHidden)
return `${braces[0]}]`;
formatter = formatTypedArray;
Expand All @@ -575,7 +617,7 @@ function formatRaw(ctx, value, recurseTimes) {
return '[Arguments] {}';
braces[0] = '[Arguments] {';
} else if (tag !== '') {
braces[0] = `${getPrefix(constructor, tag)}{`;
braces[0] = `${getPrefix(constructor, tag, 'Object')}{`;
if (keys.length === 0) {
return `${braces[0]}}`;
}
Expand Down Expand Up @@ -622,13 +664,12 @@ function formatRaw(ctx, value, recurseTimes) {
base = `[${base.slice(0, stackStart)}]`;
}
} else if (isAnyArrayBuffer(value)) {
let prefix = getPrefix(constructor, tag);
if (prefix === '') {
prefix = isArrayBuffer(value) ? 'ArrayBuffer ' : 'SharedArrayBuffer ';
}
// Fast path for ArrayBuffer and SharedArrayBuffer.
// Can't do the same for DataView because it has a non-primitive
// .buffer property that we need to recurse for.
const arrayType = isArrayBuffer(value) ? 'ArrayBuffer' :
'SharedArrayBuffer';
const prefix = getPrefix(constructor, tag, arrayType);
if (keys.length === 0)
return prefix +
`{ byteLength: ${formatNumber(ctx.stylize, value.byteLength)} }`;
Expand Down Expand Up @@ -693,9 +734,9 @@ function formatRaw(ctx, value, recurseTimes) {
} else if (keys.length === 0) {
if (isExternal(value))
return ctx.stylize('[External]', 'special');
return `${getPrefix(constructor, tag)}{}`;
return `${getPrefix(constructor, tag, 'Object')}{}`;
} else {
braces[0] = `${getPrefix(constructor, tag)}{`;
braces[0] = `${getPrefix(constructor, tag, 'Object')}{`;
}
}
}
Expand Down
98 changes: 78 additions & 20 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
const common = require('../common');
const assert = require('assert');
const { internalBinding } = require('internal/test/binding');
const { JSStream } = internalBinding('js_stream');
const JSStream = process.binding('js_stream').JSStream;
const util = require('util');
const vm = require('vm');
const { previewEntries } = internalBinding('util');
Expand Down Expand Up @@ -261,15 +261,15 @@ assert.strictEqual(
name: { value: 'Tim', enumerable: true },
hidden: { value: 'secret' }
}), { showHidden: true }),
"{ name: 'Tim', [hidden]: 'secret' }"
"[Object: null prototype] { name: 'Tim', [hidden]: 'secret' }"
);

assert.strictEqual(
util.inspect(Object.create(null, {
name: { value: 'Tim', enumerable: true },
hidden: { value: 'secret' }
})),
"{ name: 'Tim' }"
"[Object: null prototype] { name: 'Tim' }"
);

// Dynamic properties.
Expand Down Expand Up @@ -505,11 +505,17 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
set: function() {}
}
});
assert.strictEqual(util.inspect(getter, true), '{ [a]: [Getter] }');
assert.strictEqual(util.inspect(setter, true), '{ [b]: [Setter] }');
assert.strictEqual(
util.inspect(getter, true),
'[Object: null prototype] { [a]: [Getter] }'
);
assert.strictEqual(
util.inspect(setter, true),
'[Object: null prototype] { [b]: [Setter] }'
);
assert.strictEqual(
util.inspect(getterAndSetter, true),
'{ [c]: [Getter/Setter] }'
'[Object: null prototype] { [c]: [Getter/Setter] }'
);
}

Expand Down Expand Up @@ -1084,7 +1090,7 @@ if (typeof Symbol !== 'undefined') {

{
const x = Object.create(null);
assert.strictEqual(util.inspect(x), '{}');
assert.strictEqual(util.inspect(x), '[Object: null prototype] {}');
}

{
Expand Down Expand Up @@ -1224,7 +1230,7 @@ util.inspect(process);

assert.strictEqual(util.inspect(
Object.create(null, { [Symbol.toStringTag]: { value: 'foo' } })),
'[foo] {}');
'[Object: null prototype] [foo] {}');

assert.strictEqual(util.inspect(new Foo()), "Foo [bar] { foo: 'bar' }");

Expand Down Expand Up @@ -1574,20 +1580,12 @@ assert.strictEqual(util.inspect('"\''), '`"\'`');
// eslint-disable-next-line no-template-curly-in-string
assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");

// Verify the output in case the value has no prototype.
// Sadly, these cases can not be fully inspected :(
[
[/a/, '/undefined/undefined'],
[new DataView(new ArrayBuffer(2)),
'DataView {\n byteLength: undefined,\n byteOffset: undefined,\n ' +
'buffer: undefined }'],
[new SharedArrayBuffer(2), 'SharedArrayBuffer { byteLength: undefined }']
].forEach(([value, expected]) => {
{
assert.strictEqual(
util.inspect(Object.setPrototypeOf(value, null)),
expected
util.inspect(Object.setPrototypeOf(/a/, null)),
'/undefined/undefined'
);
});
}

// Verify that throwing in valueOf and having no prototype still produces nice
// results.
Expand Down Expand Up @@ -1623,6 +1621,39 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");
}
});
assert.strictEqual(util.inspect(value), expected);
value.foo = 'bar';
assert.notStrictEqual(util.inspect(value), expected);
delete value.foo;
value[Symbol('foo')] = 'yeah';
assert.notStrictEqual(util.inspect(value), expected);
});

[
[[1, 3, 4], '[Array: null prototype] [ 1, 3, 4 ]'],
[new Set([1, 2]), '[Set: null prototype] { 1, 2 }'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please also add Arrays, Dataviews and ArrayBuffers? Errors, Dates and regular expressions should be handled in a different PR as they do not show subclassing at all and that should be fixed as well.

[new Map([[1, 2]]), '[Map: null prototype] { 1 => 2 }'],
[new Promise((resolve) => setTimeout(resolve, 10)),
'[Promise: null prototype] { <pending> }'],
[new WeakSet(), '[WeakSet: null prototype] { <items unknown> }'],
[new WeakMap(), '[WeakMap: null prototype] { <items unknown> }'],
[new Uint8Array(2), '[Uint8Array: null prototype] [ 0, 0 ]'],
[new Uint16Array(2), '[Uint16Array: null prototype] [ 0, 0 ]'],
[new Uint32Array(2), '[Uint32Array: null prototype] [ 0, 0 ]'],
[new Int8Array(2), '[Int8Array: null prototype] [ 0, 0 ]'],
[new Int16Array(2), '[Int16Array: null prototype] [ 0, 0 ]'],
[new Int32Array(2), '[Int32Array: null prototype] [ 0, 0 ]'],
[new Float32Array(2), '[Float32Array: null prototype] [ 0, 0 ]'],
[new Float64Array(2), '[Float64Array: null prototype] [ 0, 0 ]'],
[new BigInt64Array(2), '[BigInt64Array: null prototype] [ 0n, 0n ]'],
[new BigUint64Array(2), '[BigUint64Array: null prototype] [ 0n, 0n ]'],
[new ArrayBuffer(16), '[ArrayBuffer: null prototype] ' +
'{ byteLength: undefined }'],
[new DataView(new ArrayBuffer(16)),
'[DataView: null prototype] {\n byteLength: undefined,\n ' +
'byteOffset: undefined,\n buffer: undefined }'],
[new SharedArrayBuffer(2), '[SharedArrayBuffer: null prototype] ' +
'{ byteLength: undefined }']
].forEach(([value, expected]) => {
assert.strictEqual(
util.inspect(Object.setPrototypeOf(value, null)),
expected
Expand Down Expand Up @@ -1706,3 +1737,30 @@ assert.strictEqual(
'[ 3, 2, 1, [Symbol(a)]: false, [Symbol(b)]: true, a: 1, b: 2, c: 3 ]'
);
}

// Manipulate the prototype to one that we can not handle.
{
let obj = { a: true };
let value = (function() { return function() {}; })();
Object.setPrototypeOf(value, null);
Object.setPrototypeOf(obj, value);
assert.strictEqual(util.inspect(obj), '{ a: true }');

obj = { a: true };
value = [];
Object.setPrototypeOf(value, null);
Object.setPrototypeOf(obj, value);
assert.strictEqual(util.inspect(obj), '{ a: true }');
}

// Check that the fallback always works.
{
const obj = new Set([1, 2]);
const iterator = obj[Symbol.iterator];
Object.setPrototypeOf(obj, null);
Object.defineProperty(obj, Symbol.iterator, {
value: iterator,
configurable: true
});
assert.strictEqual(util.inspect(obj), '[Set: null prototype] { 1, 2 }');
}