Skip to content

Commit

Permalink
util: fix proxy inspection
Browse files Browse the repository at this point in the history
This prevents any proxy traps from being called while inspecting
proxy objects. That guarantees a side-effect free way of inspecting
proxies.

PR-URL: nodejs#26241
Fixes: nodejs#10731
Fixes: nodejs#26231
Refs: nodejs#25212
Refs: nodejs#24765
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
BridgeAR committed Feb 28, 2019
1 parent 9edce1e commit a32cbe1
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 22 deletions.
24 changes: 14 additions & 10 deletions lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
Expand Down
60 changes: 48 additions & 12 deletions test/parallel/test-util-inspect-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,35 @@ const { internalBinding } = require('internal/test/binding');
const processUtil = internalBinding('util');
const opts = { showProxy: true };

const target = {};
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 handler = {
get: function() { throw new Error('Getter should not be called'); }
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'); }
};
const proxyObj = new Proxy(target, handler);
proxyObj = new Proxy(target, handler);

// Inspecting the proxy should not actually walk it's properties
util.inspect(proxyObj, opts);
Expand All @@ -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.
Expand Down

0 comments on commit a32cbe1

Please sign in to comment.