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: prevent leaking inspect internals #24971

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
4 changes: 4 additions & 0 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,10 @@ stream.write('With ES6');
<!-- YAML
added: v0.3.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/24971
description: Internal properties no longer appear in the context argument
of a custom inspection function.
- version: v11.7.0
pr-url: https://github.com/nodejs/node/pull/25006
description: ArrayBuffers now also show their binary contents.
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ class TextEncoder {
[inspect](depth, opts) {
validateEncoder(this);
if (typeof depth === 'number' && depth < 0)
return opts.stylize('[Object]', 'special');
return this;
var ctor = getConstructorOf(this);
var obj = Object.create({
constructor: ctor === null ? TextEncoder : ctor
Expand Down Expand Up @@ -517,7 +517,7 @@ function makeTextDecoderJS() {
[inspect](depth, opts) {
validateDecoder(this);
if (typeof depth === 'number' && depth < 0)
return opts.stylize('[Object]', 'special');
return this;
var ctor = getConstructorOf(this);
var obj = Object.create({
constructor: ctor === null ? TextDecoder : ctor
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ class URL {
}

if (typeof depth === 'number' && depth < 0)
return opts.stylize('[Object]', 'special');
return this;

jdalton marked this conversation as resolved.
Show resolved Hide resolved
var ctor = getConstructorOf(this);

Expand Down
8 changes: 6 additions & 2 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -516,18 +516,22 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
maybeCustom !== inspect &&
// Also filter out any prototype objects using the circular check.
!(value.constructor && value.constructor.prototype === value)) {
// Remove some internal properties from the options before passing it
// through to the user function. This also prevents option manipulation.
// eslint-disable-next-line no-unused-vars
const { budget, seen, indentationLvl, ...plainCtx } = ctx;
// This makes sure the recurseTimes are reported as before while using
// a counter internally.
const depth = ctx.depth === null ? null : ctx.depth - recurseTimes;
const ret = maybeCustom.call(value, depth, ctx);
const ret = maybeCustom.call(value, depth, plainCtx);

// If the custom inspection method returned `this`, don't go into
// infinite recursion.
if (ret !== value) {
if (typeof ret !== 'string') {
return formatValue(ctx, ret, recurseTimes);
}
return ret;
return ret.replace(/\n/g, `\n${' '.repeat(ctx.indentationLvl)}`);
}
}
}
Expand Down
20 changes: 16 additions & 4 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -775,9 +775,21 @@ util.inspect({ hasOwnProperty: null });

assert.strictEqual(util.inspect(subject), "{ foo: 'bar' }");

subject[util.inspect.custom] = (depth, opts) => {
assert.strictEqual(opts.customInspectOptions, true);
};
subject[util.inspect.custom] = common.mustCall((depth, opts) => {
const clone = { ...opts };
// This might change at some point but for now we keep the stylize function.
// The function should either be documented or an alternative should be
// implemented.
assert.strictEqual(typeof opts.stylize, 'function');
assert.strictEqual(opts.seen, undefined);
assert.strictEqual(opts.budget, undefined);
assert.strictEqual(opts.indentationLvl, undefined);
assert.strictEqual(opts.showHidden, false);
opts.showHidden = true;
return { [util.inspect.custom]: common.mustCall((depth, opts2) => {
assert.deepStrictEqual(clone, opts2);
}) };
});

util.inspect(subject, { customInspectOptions: true });

Expand Down Expand Up @@ -1584,7 +1596,7 @@ util.inspect(process);
);
const longList = util.inspect(list, { depth: Infinity });
const match = longList.match(/next/g);
assert(match.length > 1000 && match.length < 10000);
assert(match.length > 500 && match.length < 10000);
assert(longList.includes('[Object: Inspection interrupted ' +
'prematurely. Maximum call stack size exceeded.]'));
}
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-whatwg-encoding-custom-textdecoder.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ if (common.hasIntl) {
// Test TextDecoder inspect with negative depth
{
const dec = new TextDecoder();
assert.strictEqual(util.inspect(dec, { depth: -1 }), '[Object]');
assert.strictEqual(util.inspect(dec, { depth: -1 }), '[TextDecoder]');
}

{
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-whatwg-url-custom-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ assert.strictEqual(

assert.strictEqual(
util.inspect({ a: url }, { depth: 0 }),
'{ a: [Object] }');
'{ a: [URL] }');

class MyURL extends URL {}
assert(util.inspect(new MyURL(url.href)).startsWith('MyURL {'));