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: fix infinite recursion during inspection #37079

Closed
wants to merge 2 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
24 changes: 10 additions & 14 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -557,12 +557,11 @@ changes:
(in combination with `compact` set to `true` or any number >= `1`).
**Default:** `80`.
* `compact` {boolean|integer} Setting this to `false` causes each object key
to be displayed on a new line. It will also add new lines to text that is
to be displayed on a new line. It will break on new lines in text that is
longer than `breakLength`. If set to a number, the most `n` inner elements
are united on a single line as long as all properties fit into
`breakLength`. Short array elements are also grouped together. No
text will be reduced below 16 characters, no matter the `breakLength` size.
For more information, see the example below. **Default:** `3`.
`breakLength`. Short array elements are also grouped together. For more
information, see the example below. **Default:** `3`.
* `sorted` {boolean|Function} If set to `true` or a function, all properties
of an object, and `Set` and `Map` entries are sorted in the resulting
string. If set to `true` the [default sort][] is used. If set to a function,
Expand Down Expand Up @@ -630,8 +629,8 @@ const util = require('util');

const o = {
a: [1, 2, [[
'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do ' +
'eiusmod tempor incididunt ut labore et dolore magna aliqua.',
'Lorem ipsum dolor sit amet,\nconsectetur adipiscing elit, sed do ' +
'eiusmod \ntempor incididunt ut labore et dolore magna aliqua.',
'test',
'foo']], 4],
b: new Map([['za', 1], ['zb', 'test']])
Expand All @@ -641,13 +640,13 @@ console.log(util.inspect(o, { compact: true, depth: 5, breakLength: 80 }));
// { a:
// [ 1,
// 2,
// [ [ 'Lorem ipsum dolor sit amet, consectetur [...]', // A long line
// [ [ 'Lorem ipsum dolor sit amet,\nconsectetur [...]', // A long line
// 'test',
// 'foo' ] ],
// 4 ],
// b: Map(2) { 'za' => 1, 'zb' => 'test' } }

// Setting `compact` to false changes the output to be more reader friendly.
// Setting `compact` to false or an integer creates more reader friendly output.
console.log(util.inspect(o, { compact: false, depth: 5, breakLength: 80 }));

// {
Expand All @@ -656,10 +655,9 @@ console.log(util.inspect(o, { compact: false, depth: 5, breakLength: 80 }));
// 2,
// [
// [
// 'Lorem ipsum dolor sit amet, consectetur ' +
// 'adipiscing elit, sed do eiusmod tempor ' +
// 'incididunt ut labore et dolore magna ' +
// 'aliqua.,
// 'Lorem ipsum dolor sit amet,\n' +
// 'consectetur adipiscing elit, sed do eiusmod \n' +
// 'tempor incididunt ut labore et dolore magna aliqua.',
// 'test',
// 'foo'
// ]
Expand All @@ -674,8 +672,6 @@ console.log(util.inspect(o, { compact: false, depth: 5, breakLength: 80 }));

// Setting `breakLength` to e.g. 150 will print the "Lorem ipsum" text in a
// single line.
// Reducing the `breakLength` will split the "Lorem ipsum" text in smaller
// chunks.
```

The `showHidden` option allows [`WeakMap`][] and [`WeakSet`][] entries to be
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
ArrayIsArray,
ArrayPrototypeFilter,
ArrayPrototypeForEach,
ArrayPrototypePop,
ArrayPrototypePush,
ArrayPrototypePushApply,
ArrayPrototypeSort,
Expand Down Expand Up @@ -619,6 +620,7 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
}
// Get all own property names and symbols.
keys = ReflectOwnKeys(obj);
ArrayPrototypePush(ctx.seen, main);
for (const key of keys) {
// Ignore the `constructor` property and keys that exist on layers above.
if (key === 'constructor' ||
Expand All @@ -639,6 +641,7 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
ArrayPrototypePush(output, value);
}
}
ArrayPrototypePop(ctx.seen);
// Limit the inspection to up to three prototype layers. Using `recurseTimes`
// is not a good choice here, because it's as if the properties are declared
// on the current object from the users perspective.
Expand Down
67 changes: 52 additions & 15 deletions test/parallel/test-util-inspect-getters-accessing-this.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,61 @@ require('../common');

const assert = require('assert');

const util = require('util');
const { inspect } = require('util');

class X {
constructor() {
this._y = 123;
}
{
class X {
constructor() {
this._y = 123;
}

get y() {
return this._y;
get y() {
return this._y;
}
}

const result = inspect(new X(), {
getters: true,
showHidden: true
});

assert.strictEqual(
result,
'X { _y: 123, [y]: [Getter: 123] }'
);
}

const result = util.inspect(new X(), {
getters: true,
showHidden: true
});
// Regression test for https://github.com/nodejs/node/issues/37054
{
class A {
constructor(B) {
this.B = B;
}
get b() {
return this.B;
}
}

class B {
constructor() {
this.A = new A(this);
}
get a() {
return this.A;
}
}

const result = inspect(new B(), {
depth: 1,
getters: true,
showHidden: true
});

assert.strictEqual(
result,
'X { _y: 123, [y]: [Getter: 123] }'
);
assert.strictEqual(
result,
'<ref *1> B {\n' +
' A: A { B: [Circular *1], [b]: [Getter] [Circular *1] },\n' +
' [a]: [Getter] A { B: [Circular *1], [b]: [Getter] [Circular *1] }\n' +
'}',
);
}