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 inspection output size to 128 MB #22756

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
15 changes: 10 additions & 5 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,10 @@ stream.write('With ES6');
<!-- YAML
added: v0.3.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/REPLACEME
description: The inspection output is now limited to 128 MB. Data above that
size not be inspected fully anymore.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is missing a verb? 😄

- version: v10.6.0
pr-url: https://github.com/nodejs/node/pull/20725
description: Inspecting linked lists and similar objects is now possible
Expand Down Expand Up @@ -408,11 +412,11 @@ changes:
TODO(BridgeAR): Deprecate `maxArrayLength` and replace it with
`maxEntries`.
-->
* `maxArrayLength` {number} Specifies the maximum number of `Array`,
* `maxArrayLength` {integer} 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
negative to show no elements. **Default:** `100`.
* `breakLength` {number} The length at which an object's keys are split
* `breakLength` {integer} The length at which an object's keys are split
across multiple lines. Set to `Infinity` to format an object as a single
line. **Default:** `60` for legacy compatibility.
* `compact` {boolean} Setting this to `false` changes the default indentation
Expand Down Expand Up @@ -532,9 +536,10 @@ console.log(inspect(weakSet, { showHidden: true }));
```

Please note that `util.inspect()` is a synchronous method that is mainly
intended as a debugging tool. Some input values can have a significant
performance overhead that can block the event loop. Use this function
with care and never in a hot code path.
intended as a debugging tool. Its maximum output length is limited to 128 MB and
input values that result in output bigger than that will not be inspected fully.
Copy link
Member

Choose a reason for hiding this comment

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

☝️ Since this is a heuristic is it approximately 128 mb or exactly?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be detected by the very first entry that exceeds that limit (but only after being fully inspected at that level).

Copy link
Member Author

Choose a reason for hiding this comment

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

And that is also not exact as the higher levels could already contain data that will be added on top of that. It is fairly precise if the individual properties on the object are not that big. If each of them is huge, the limit will not be as exact.

Copy link
Member

Choose a reason for hiding this comment

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

Could the wording be tweaked to qualify it as "approximately 128 MB"?

Such values can have a significant performance overhead that can block the event
loop for a significant amount of time.

### Customizing `util.inspect` colors

Expand Down
62 changes: 39 additions & 23 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,24 +406,27 @@ function inspect(value, opts) {
maxArrayLength: inspectDefaultOptions.maxArrayLength,
breakLength: inspectDefaultOptions.breakLength,
indentationLvl: 0,
compact: inspectDefaultOptions.compact
compact: inspectDefaultOptions.compact,
budget: {}
};
// Legacy...
if (arguments.length > 2) {
if (arguments[2] !== undefined) {
ctx.depth = arguments[2];
}
if (arguments.length > 3 && arguments[3] !== undefined) {
ctx.colors = arguments[3];
if (arguments.length > 1) {
// Legacy...
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
if (arguments.length > 2) {
if (arguments[2] !== undefined) {
ctx.depth = arguments[2];
}
if (arguments.length > 3 && arguments[3] !== undefined) {
ctx.colors = arguments[3];
}
}
}
// Set user-specified options
if (typeof opts === 'boolean') {
ctx.showHidden = opts;
} else if (opts) {
const optKeys = Object.keys(opts);
for (var i = 0; i < optKeys.length; i++) {
ctx[optKeys[i]] = opts[optKeys[i]];
// Set user-specified options
if (typeof opts === 'boolean') {
ctx.showHidden = opts;
} else if (opts) {
const optKeys = Object.keys(opts);
for (var i = 0; i < optKeys.length; i++) {
ctx[optKeys[i]] = opts[optKeys[i]];
}
}
}
if (ctx.colors) ctx.stylize = stylizeWithColor;
Expand Down Expand Up @@ -623,14 +626,19 @@ function noPrototypeIterator(ctx, value, recurseTimes) {
// corrected by setting `ctx.indentationLvL += diff` and then to decrease the
// value afterwards again.
function formatValue(ctx, value, recurseTimes) {
// Primitive types cannot have properties
// Primitive types cannot have properties.
if (typeof value !== 'object' && typeof value !== 'function') {
return formatPrimitive(ctx.stylize, value, ctx);
}
if (value === null) {
return ctx.stylize('null', 'null');
}

if (ctx.stop) {
const name = getConstructorName(value) || value[Symbol.toStringTag];
return ctx.stylize(`[${name || 'Object'}]`, 'special');
}

if (ctx.showProxy) {
const proxy = getProxyDetails(value);
if (proxy !== undefined) {
Expand All @@ -639,11 +647,11 @@ function formatValue(ctx, value, recurseTimes) {
}

// Provide a hook for user-specified inspect functions.
// Check that value is an object with an inspect function on it
// Check that value is an object with an inspect function on it.
if (ctx.customInspect) {
const maybeCustom = value[customInspectSymbol];
if (typeof maybeCustom === 'function' &&
// Filter out the util module, its inspect function is special
// Filter out the util module, its inspect function is special.
maybeCustom !== exports.inspect &&
// Also filter out any prototype objects using the circular check.
!(value.constructor && value.constructor.prototype === value)) {
Expand Down Expand Up @@ -685,7 +693,7 @@ function formatRaw(ctx, value, recurseTimes) {

let extrasType = kObjectType;

// Iterators and the rest are split to reduce checks
// Iterators and the rest are split to reduce checks.
if (value[Symbol.iterator]) {
noIterator = false;
if (Array.isArray(value)) {
Expand Down Expand Up @@ -766,7 +774,7 @@ function formatRaw(ctx, value, recurseTimes) {
}
base = dateToISOString(value);
} else if (isError(value)) {
// Make error with message first say the error
// Make error with message first say the error.
base = formatError(value);
// Wrap the error in brackets in case it has no stack trace.
const stackStart = base.indexOf('\n at');
Expand Down Expand Up @@ -885,7 +893,14 @@ function formatRaw(ctx, value, recurseTimes) {
}
ctx.seen.pop();

return reduceToSingleString(ctx, output, base, braces);
const res = reduceToSingleString(ctx, output, base, braces);
const budget = ctx.budget[ctx.indentationLvl] || 0;
const newLength = budget + res.length;
ctx.budget[ctx.indentationLvl] = newLength;
if (newLength > 2 ** 27) {
Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment here?

ctx.stop = true;
}
return res;
}
Copy link
Member

Choose a reason for hiding this comment

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

I get wanting to limit string construction to avoid memory issues and it looks like when this is implemented objects just won't be crawled (something like "[Object]") which are both good.

  • Am I correct that the indentation level is just a hint that there could be a possible problem (it's unknown until the objects are crawled and inspected) and so it's being defensive and not going further?

  • Can indentation level be translated to depth?
    (I think depth would be easier to reason about at a glance than indenting)

  • Can an object still be created that busts this heuristic?

Copy link
Member Author

@BridgeAR BridgeAR Sep 12, 2018

Choose a reason for hiding this comment

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

  • Yes, it's unknown until we crawled and inspected the object and after exceeding the limit it's continuing defensively.
  • Yes, that is possible. Using depth is not possible though, as it's value is set by the user (e.g., to Infinity), not by inspect() itself. indentationLvl is only meant to be internal and is never set to Infinity.
  • That would be possible. E.g., util.inspect(new Uint8Array(1e8), { maxArrayLength: Infinity }). If an individual part of an object exceeds the limit on it's own or the total space left, it's not going to protect against it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at this again: the depth and the indentationLvl would be great to be combined but the compact: true mode has different indention for different values. That's why it's currently not possible :/

Copy link
Member

Choose a reason for hiding this comment

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

That's cool. This is a good first attempt at least.


function handleMaxCallStackSize(ctx, err, constructor, tag) {
Expand Down Expand Up @@ -1057,8 +1072,9 @@ function formatTypedArray(ctx, value, recurseTimes) {
formatBigInt;
for (var i = 0; i < maxLength; ++i)
output[i] = elementFormatter(ctx.stylize, value[i]);
if (remaining > 0)
if (remaining > 0) {
output[i] = `... ${remaining} more item${remaining > 1 ? 's' : ''}`;
}
if (ctx.showHidden) {
// .buffer goes last, it's not a primitive like the others.
ctx.indentationLvl += 2;
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-util-inspect-long-running.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';

require('../common');

// Test that huge objects don't crash due to exceeding the maximum heap size.

const util = require('util');

// Create a difficult to stringify object. Without the artificial limitation
// this would crash or throw an maximum string size error.
let last = {};
const obj = last;

for (let i = 0; i < 1000; i++) {
last.next = { circular: obj, last, obj: { a: 1, b: 2, c: true } };
last = last.next;
obj[i] = last;
}

util.inspect(obj, { depth: Infinity });