From 5672ab766829d8808afe5d34eb78149d3ee8c51e Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 10 Mar 2019 23:59:49 +0100 Subject: [PATCH] util: prevent tampering with internals in `inspect()` This makes sure user options passed to `util.inspect()` will not override any internal properties (besides `stylize`). PR-URL: https://github.com/nodejs/node/pull/26577 Fixes: https://github.com/nodejs/node/issues/24765 Reviewed-By: Matteo Collina Reviewed-By: Rich Trott --- lib/internal/util/inspect.js | 28 +++++++++++++++++++++------- test/parallel/test-util-inspect.js | 12 ++++++++++-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index b30a4e4a4ae53a..4cee2d8b9fb4a3 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -150,6 +150,16 @@ const meta = [ '', '', '', '', '', '', '', '\\\\' ]; +function getUserOptions(ctx) { + const obj = { stylize: ctx.stylize }; + for (const key of Object.keys(inspectDefaultOptions)) { + obj[key] = ctx[key]; + } + if (ctx.userOptions === undefined) + return obj; + return { ...obj, ...ctx.userOptions }; +} + /** * Echos the value of any input. Tries to print the value out * in the best way possible given the different types. @@ -192,8 +202,16 @@ function inspect(value, opts) { 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]]; + for (const key of optKeys) { + // TODO(BridgeAR): Find a solution what to do about stylize. Either make + // this function public or add a new API with a similar or better + // functionality. + if (hasOwnProperty(inspectDefaultOptions, key) || key === 'stylize') { + ctx[key] = opts[key]; + } else if (ctx.userOptions === undefined) { + // This is required to pass through the actual user input. + ctx.userOptions = opts; + } } } } @@ -522,14 +540,10 @@ 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(context, depth, plainCtx); + const ret = maybeCustom.call(context, depth, getUserOptions(ctx)); // If the custom inspection method returned `this`, don't go into // infinite recursion. if (ret !== context) { diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index c47d15e0863924..9abbeec199b09a 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -791,7 +791,7 @@ util.inspect({ hasOwnProperty: null }); }) }; }); - util.inspect(subject, { customInspectOptions: true }); + util.inspect(subject); // util.inspect.custom is a shared symbol which can be accessed as // Symbol.for("nodejs.util.inspect.custom"). @@ -803,9 +803,11 @@ util.inspect({ hasOwnProperty: null }); subject[inspect] = (depth, opts) => { assert.strictEqual(opts.customInspectOptions, true); + assert.strictEqual(opts.seen, null); + return {}; }; - util.inspect(subject, { customInspectOptions: true }); + util.inspect(subject, { customInspectOptions: true, seen: null }); } { @@ -816,6 +818,12 @@ util.inspect({ hasOwnProperty: null }); `{ a: 123,\n [Symbol(${UIC})]: [Function: [${UIC}]] }`); } +// Verify that it's possible to use the stylize function to manipulate input. +assert.strictEqual( + util.inspect([1, 2, 3], { stylize() { return 'x'; } }), + '[ x, x, x ]' +); + // Using `util.inspect` with "colors" option should produce as many lines as // without it. {