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: limit util inspect depth to 10 #20627

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
13 changes: 7 additions & 6 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,13 @@ stream.write('With ES6');
<!-- YAML
added: v0.3.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/REPLACEME
description: The `depth` default changed to 10.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: `10`?

- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/19259
description: The `WeakMap` and `WeakSet` entries can now be inspected
as well.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/17907
description: The `depth` default changed to `Infinity`.
- version: v9.9.0
pr-url: https://github.com/nodejs/node/pull/17576
description: The `compact` option is supported now.
Expand Down Expand Up @@ -407,7 +407,7 @@ changes:
-->
* `maxArrayLength` {number} Specifies the maximum number of `Array`,
[`TypedArray`][], [`WeakMap`][] and [`WeakSet`][] elements to include when
formatting. Set to `null` or `Infinity` to show all elements. Set to `0` or
formatting. Set to `Infinity` or `null` to show all elements. Set to `0` or
negative to show no elements. **Default:** `100`.
* `breakLength` {number} The length at which an object's keys are split
across multiple lines. Set to `Infinity` to format an object as a single
Expand All @@ -421,8 +421,9 @@ changes:
example below. **Default:** `true`.
* `depth` {number} Specifies the number of visible nested `Object`s in an
`object`. This is useful to minimize the inspection output for large
complicated objects. To make it recurse indefinitely pass `null` or
`Infinity`. **Default:** `Infinity`.
complicated objects. To make it recurse almost indefinitely pass any high
number, `Infinity` or `null`. The real value is capped at 1000.
**Default:** `10`.
* Returns: {string} The representation of passed object

The `util.inspect()` method returns a string representation of `object` that is
Expand Down
29 changes: 21 additions & 8 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,9 @@ function dnsException(code, syscall, hostname) {
return ex;
}

let maxStack_ErrorName;
let maxStack_ErrorMessage;
let maxStackErrorName;
let maxStackErrorMessage;
let maxStackLimit = 0;
/**
* Returns true if `err.name` and `err.message` are equal to engine-specific
* values indicating max call stack size has been exceeded.
Expand All @@ -669,18 +670,29 @@ let maxStack_ErrorMessage;
* @returns {boolean}
*/
function isStackOverflowError(err) {
if (maxStack_ErrorMessage === undefined) {
if (maxStackErrorMessage === undefined) {
try {
function overflowStack() { overflowStack(); }
function overflowStack() {
maxStackLimit++;
overflowStack();
}
overflowStack();
} catch (err) {
maxStack_ErrorMessage = err.message;
maxStack_ErrorName = err.name;
maxStackErrorMessage = err.message;
maxStackErrorName = err.name;
}
}

return err.name === maxStack_ErrorName &&
err.message === maxStack_ErrorMessage;
return err.name === maxStackErrorName &&
err.message === maxStackErrorMessage;
}

function getMaxStackLimit() {
if (maxStackLimit === 0) {
// Use a fake "error"
isStackOverflowError({ name: '', message: '' });
}
return maxStackLimit;
Copy link
Member

Choose a reason for hiding this comment

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

I’m not sure, but I wouldn’t expect that stack overflows happen all at the same depths – some stack frames will necessarily take more space than others, so what this would give us is an upper bound rather than something that is necessarily usable.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is correct. That is also why I do not directly use it. In the end I use a absolute limit of 1000. That could be lowered if this function returns a low number. That is all I do with that. But I am perfectly fine with just always using 1000 as limit and to remove this again.

Copy link
Member

Choose a reason for hiding this comment

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

That would be fine with me, yea. (But then again depth: null kinda sounds like opting into accepting stack overflow errors to me 😄)

Copy link
Member Author

Choose a reason for hiding this comment

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

I also pointed out above that I have a working patch that allows to inspect really deep nested objects without such errors. With that patch applied we could set a absolute depth limit of e.g. 10000 (anything above that is going to cause a lot of CPU time and the heap memory will not be enough at some point). What I do is to catch the error, bubble to the top and start over with the part that caused the issue and than assemble everything.

}

module.exports = {
Expand All @@ -689,6 +701,7 @@ module.exports = {
exceptionWithHostPort,
uvException,
isStackOverflowError,
getMaxStackLimit,
getMessage,
AssertionError,
SystemError,
Expand Down
44 changes: 30 additions & 14 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@

'use strict';

const errors = require('internal/errors');
const {
ERR_FALSY_VALUE_REJECTION,
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE
} = errors.codes;
getMaxStackLimit,
errnoException: _errnoException,
exceptionWithHostPort: _exceptionWithHostPort,
codes: {
ERR_FALSY_VALUE_REJECTION,
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE
}
} = require('internal/errors');
const { TextDecoder, TextEncoder } = require('internal/encoding');
const { isBuffer } = require('buffer').Buffer;

Expand Down Expand Up @@ -80,7 +84,7 @@ const {

const inspectDefaultOptions = Object.seal({
showHidden: false,
depth: null,
depth: 10,
colors: false,
customInspect: true,
showProxy: false,
Expand All @@ -105,6 +109,12 @@ const numberRegExp = /^(0|[1-9][0-9]*)$/;

const readableRegExps = {};

// The actual maximum stack limit has to be divided by the number of function
// calls that are maximal necessary for each depth level. That does not seem
// to be sufficient though, so let us just divide it by 10 and use 1000 as upper
// limit.
const MAX_DEPTH_LIMIT = Math.min(Math.floor(getMaxStackLimit() / 10), 1000);
Copy link
Member

Choose a reason for hiding this comment

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

This still seems too magical to me - no less magical than the depth: 2 default at least, but that's simpler.


const MIN_LINE_LENGTH = 16;

// Escaped special characters. Use empty strings to fill up unused entries.
Expand Down Expand Up @@ -287,6 +297,14 @@ function debuglog(set) {
return debugs[set];
}

function getMaxDepth(depth) {
if (depth === null)
return MAX_DEPTH_LIMIT;
if (depth < MAX_DEPTH_LIMIT)
return depth;
return MAX_DEPTH_LIMIT;
}

/**
* Echos the value of any input. Tries to print the value out
* in the best way possible given the different types.
Expand Down Expand Up @@ -332,7 +350,7 @@ function inspect(value, opts) {
}
if (ctx.colors) ctx.stylize = stylizeWithColor;
if (ctx.maxArrayLength === null) ctx.maxArrayLength = Infinity;
return formatValue(ctx, value, ctx.depth);
return formatValue(ctx, value, getMaxDepth(ctx.depth));
}
inspect.custom = customInspectSymbol;

Expand Down Expand Up @@ -677,11 +695,9 @@ function formatValue(ctx, value, recurseTimes, ln) {
}
}

if (recurseTimes != null) {
if (recurseTimes < 0)
return ctx.stylize(`[${constructor || tag || 'Object'}]`, 'special');
recurseTimes -= 1;
}
if (recurseTimes < 0)
return ctx.stylize(`[${constructor || tag || 'Object'}]`, 'special');
recurseTimes -= 1;

ctx.seen.push(value);
const output = formatter(ctx, value, recurseTimes, keys);
Expand Down Expand Up @@ -1246,8 +1262,8 @@ function getSystemErrorName(err) {

// Keep the `exports =` so that various functions can still be monkeypatched
module.exports = exports = {
_errnoException: errors.errnoException,
_exceptionWithHostPort: errors.exceptionWithHostPort,
_errnoException,
_exceptionWithHostPort,
_extend,
callbackify,
debuglog,
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -1404,3 +1404,21 @@ util.inspect(process);
const args = (function() { return arguments; })('a');
assert.strictEqual(util.inspect(args), "[Arguments] { '0': 'a' }");
}

{
// Test that a long linked list can be inspected without throwing an error.
const list = {};
let head = list;
// A linked list of length 100k should be inspectable in some way, even though
// the real cutoff value is much lower than 100k.
for (let i = 0; i < 100000; i++)
head = head.next = {};
assert.strictEqual(
util.inspect(list),
'{ next: \n { next: \n { next: \n { next: \n { ' +
'next: \n { next: { next: { next: { next: { next: { ' +
'next: [Object] } } } } } } } } } } }'
);
const longList = util.inspect(list, { depth: Infinity });
assert.strictEqual(longList.match(/next/g).length, 1001);
}