From 7bbc265dad8ac63f846bcf27315f9c37f263ca66 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 10 Mar 2019 23:59:49 +0100 Subject: [PATCH 1/3] util: prevent tampering with internals in `inspect()` This makes sure user options passed to `util.inspect()` will not override any internal properties (besides `stylize`). --- lib/internal/util/inspect.js | 28 +++++++++++++++++++++------- test/parallel/test-util-inspect.js | 6 ++++-- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index b30a4e4a4ae53a..65d50cd07a10eb 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..d64fa45da4ffc3 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 }); } { From 21c0e3e7f7ee27e476c1458c323a0013a0ec42bd Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 12 Mar 2019 18:28:23 +0100 Subject: [PATCH 2/3] util: prevent tampering with internals in `inspect()` --- lib/internal/util/inspect.js | 2 +- test/parallel/test-util-inspect.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 65d50cd07a10eb..4cee2d8b9fb4a3 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -206,7 +206,7 @@ function inspect(value, opts) { // 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') { + 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. diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index d64fa45da4ffc3..66b2c6dd447a0c 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -818,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 ]' +); + // Using `util.inspect` with "colors" option should produce as many lines as // without it. { From 6d65b57042543a21f50e284f5e7e006d68e50e2f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 12 Mar 2019 20:09:02 +0100 Subject: [PATCH 3/3] fixup! test --- test/parallel/test-util-inspect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 66b2c6dd447a0c..9abbeec199b09a 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -820,7 +820,7 @@ util.inspect({ hasOwnProperty: null }); // Verify that it's possible to use the stylize function to manipulate input. assert.strictEqual( - util.inspect([1, 2, 3], { stylize() { return ''; } }), + util.inspect([1, 2, 3], { stylize() { return 'x'; } }), '[ x, x, x ]' );