util: fix proxy inspection

This prevents any proxy traps from being called while inspecting
proxy objects. That guarantees a side-effect free way of inspecting
proxies.

PR-URL: https://github.com/nodejs/node/pull/26241
Fixes: https://github.com/nodejs/node/issues/10731
Fixes: https://github.com/nodejs/node/issues/26231
Refs: https://github.com/nodejs/node/issues/25212
Refs: https://github.com/nodejs/node/issues/24765
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Ruben Bridgewater 2019-02-21 16:07:07 +01:00
parent 9edce1e12a
commit a32cbe1597
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
2 changed files with 63 additions and 23 deletions

View File

@ -495,18 +495,23 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
return ctx.stylize('null', 'null');
}
// Memorize the context for custom inspection on proxies.
const context = value;
// Always check for proxies to prevent side effects and to prevent triggering
// any proxy handlers.
const proxy = getProxyDetails(value);
if (proxy !== undefined) {
if (ctx.showProxy && ctx.stop === undefined) {
return formatProxy(ctx, proxy, recurseTimes);
}
value = proxy[0];
}
if (ctx.stop !== undefined) {
const name = getConstructorName(value, ctx) || value[Symbol.toStringTag];
return ctx.stylize(`[${name || 'Object'}]`, 'special');
}
if (ctx.showProxy) {
const proxy = getProxyDetails(value);
if (proxy !== undefined) {
return formatProxy(ctx, proxy, recurseTimes);
}
}
// Provide a hook for user-specified inspect functions.
// Check that value is an object with an inspect function on it.
if (ctx.customInspect) {
@ -523,11 +528,10 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
// 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, plainCtx);
const ret = maybeCustom.call(context, depth, plainCtx);
// If the custom inspection method returned `this`, don't go into
// infinite recursion.
if (ret !== value) {
if (ret !== context) {
if (typeof ret !== 'string') {
return formatValue(ctx, ret, recurseTimes);
}

View File

@ -8,11 +8,35 @@ const { internalBinding } = require('internal/test/binding');
const processUtil = internalBinding('util');
const opts = { showProxy: true };
const target = {};
const handler = {
get: function() { throw new Error('Getter should not be called'); }
let proxyObj;
let called = false;
const target = {
[util.inspect.custom](depth, { showProxy }) {
if (showProxy === false) {
called = true;
if (proxyObj !== this) {
throw new Error('Failed');
}
}
return [1, 2, 3];
}
};
const proxyObj = new Proxy(target, handler);
const handler = {
getPrototypeOf() { throw new Error('getPrototypeOf'); },
setPrototypeOf() { throw new Error('setPrototypeOf'); },
isExtensible() { throw new Error('isExtensible'); },
preventExtensions() { throw new Error('preventExtensions'); },
getOwnPropertyDescriptor() { throw new Error('getOwnPropertyDescriptor'); },
defineProperty() { throw new Error('defineProperty'); },
has() { throw new Error('has'); },
get() { throw new Error('get'); },
set() { throw new Error('set'); },
deleteProperty() { throw new Error('deleteProperty'); },
ownKeys() { throw new Error('ownKeys'); },
apply() { throw new Error('apply'); },
construct() { throw new Error('construct'); }
};
proxyObj = new Proxy(target, handler);
// Inspecting the proxy should not actually walk it's properties
util.inspect(proxyObj, opts);
@ -23,19 +47,31 @@ const details = processUtil.getProxyDetails(proxyObj);
assert.strictEqual(target, details[0]);
assert.strictEqual(handler, details[1]);
assert.strictEqual(util.inspect(proxyObj, opts),
'Proxy [ {}, { get: [Function: get] } ]');
assert.strictEqual(
util.inspect(proxyObj, opts),
'Proxy [ [ 1, 2, 3 ],\n' +
' { getPrototypeOf: [Function: getPrototypeOf],\n' +
' setPrototypeOf: [Function: setPrototypeOf],\n' +
' isExtensible: [Function: isExtensible],\n' +
' preventExtensions: [Function: preventExtensions],\n' +
' getOwnPropertyDescriptor: [Function: getOwnPropertyDescriptor],\n' +
' defineProperty: [Function: defineProperty],\n' +
' has: [Function: has],\n' +
' get: [Function: get],\n' +
' set: [Function: set],\n' +
' deleteProperty: [Function: deleteProperty],\n' +
' ownKeys: [Function: ownKeys],\n' +
' apply: [Function: apply],\n' +
' construct: [Function: construct] } ]'
);
// Using getProxyDetails with non-proxy returns undefined
assert.strictEqual(processUtil.getProxyDetails({}), undefined);
// This will throw because the showProxy option is not used
// and the get function on the handler object defined above
// is actually invoked.
assert.throws(
() => util.inspect(proxyObj),
/^Error: Getter should not be called$/
);
// Inspecting a proxy without the showProxy option set to true should not
// trigger any proxy handlers.
assert.strictEqual(util.inspect(proxyObj), '[ 1, 2, 3 ]');
assert(called);
// Yo dawg, I heard you liked Proxy so I put a Proxy
// inside your Proxy that proxies your Proxy's Proxy.