util: never trigger proxy traps while inspecting objects

benchmark/util/format.js

@@ -13,6 +13,8 @@ const inputs = {
   'no-replace-2': ['foobar', 'yeah', 'mensch', 5],
   'only-objects': [{ msg: 'This is an error' }, { msg: 'This is an error' }],
   'many-%': ['replace%%%%s%%%%many%s%s%s', 'percent'],
+  'object-to-string': ['foo %s bar', { toString() { return 'bla'; } }],
+  'object-%s': ['foo %s bar', { a: true, b: false }],
 };
 
 const bench = common.createBenchmark(main, {

benchmark/util/inspect-proxy.js

@@ -3,13 +3,21 @@
 const util = require('util');
 const common = require('../common.js');
 
-const bench = common.createBenchmark(main, { n: [2e4] });
+const bench = common.createBenchmark(main, {
+  n: [1e5],
+  showProxy: [0, 1],
+  isProxy: [0, 1]
+});
 
-function main({ n }) {
-  const proxyA = new Proxy({}, { get: () => {} });
-  const proxyB = new Proxy(() => {}, {});
+function main({ n, showProxy, isProxy }) {
+  let proxyA = {};
+  let proxyB = () => {};
+  if (isProxy) {
+    proxyA = new Proxy(proxyA, { get: () => {} });
+    proxyB = new Proxy(proxyB, {});
+  }
   bench.start();
   for (let i = 0; i < n; i += 1)
-    util.inspect({ a: proxyA, b: proxyB }, { showProxy: true });
+    util.inspect({ a: proxyA, b: proxyB }, { showProxy });
   bench.end(n);
 }

lib/internal/util/inspect.js

@@ -570,12 +570,12 @@ function formatValue(ctx, value, recurseTimes, typedArray) {
   const context = value;
   // Always check for proxies to prevent side effects and to prevent triggering
   // any proxy handlers.
-  const proxy = getProxyDetails(value);
+  const proxy = getProxyDetails(value, !!ctx.showProxy);
   if (proxy !== undefined) {
     if (ctx.showProxy) {
       return formatProxy(ctx, proxy, recurseTimes);
     }
-    value = proxy[0];
+    value = proxy;
   }
 
   // Provide a hook for user-specified inspect functions.
@@ -1575,6 +1575,13 @@ function reduceToSingleString(
 }
 
 function hasBuiltInToString(value) {
+  // Prevent triggering proxy traps.
+  const getFullProxy = false;
+  const proxyTarget = getProxyDetails(value, getFullProxy);
+  if (proxyTarget !== undefined) {
+    value = proxyTarget;
+  }
+
   // Count objects that have no `toString` function as built-in.
   if (typeof value.toString !== 'function') {
     return true;

src/node_util.cc

@@ -91,15 +91,23 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
   if (!args[0]->IsProxy())
     return;
 
+  CHECK(args[1]->IsBoolean());
+
   Local<Proxy> proxy = args[0].As<Proxy>();
 
-  Local<Value> ret[] = {
-    proxy->GetTarget(),
-    proxy->GetHandler()
-  };
+  if (args[1]->IsTrue()) {
+    Local<Value> ret[] = {
+      proxy->GetTarget(),
+      proxy->GetHandler()
+    };
 
-  args.GetReturnValue().Set(
-      Array::New(args.GetIsolate(), ret, arraysize(ret)));
+    args.GetReturnValue().Set(
+        Array::New(args.GetIsolate(), ret, arraysize(ret)));
+  } else {
+    Local<Value> ret = proxy->GetTarget();
+
+    args.GetReturnValue().Set(ret);
+  }
 }
 
 static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {

test/benchmark/test-benchmark-util.js

@@ -14,5 +14,7 @@ runBenchmark('util',
               'size=1',
               'type=',
               'len=1',
-              'version=native'],
+              'version=native',
+              'isProxy=1',
+              'showProxy=1'],
              { NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });

test/parallel/test-util-inspect-proxy.js

@@ -41,12 +41,18 @@ proxyObj = new Proxy(target, handler);
 // Inspecting the proxy should not actually walk it's properties
 util.inspect(proxyObj, opts);
 
+// Make sure inspecting object does not trigger any proxy traps.
+util.format('%s', proxyObj);
+
 // getProxyDetails is an internal method, not intended for public use.
 // This is here to test that the internals are working correctly.
-const details = processUtil.getProxyDetails(proxyObj);
+let details = processUtil.getProxyDetails(proxyObj, true);
 assert.strictEqual(target, details[0]);
 assert.strictEqual(handler, details[1]);
 
+details = processUtil.getProxyDetails(proxyObj, false);
+assert.strictEqual(target, details);
+
 assert.strictEqual(
   util.inspect(proxyObj, opts),
   'Proxy [\n' +
@@ -105,7 +111,7 @@ const expected6 = 'Proxy [\n' +
                   '  ]\n' +
                   ']';
 assert.strictEqual(
-  util.inspect(proxy1, { showProxy: true, depth: null }),
+  util.inspect(proxy1, { showProxy: 1, depth: null }),
   expected1);
 assert.strictEqual(util.inspect(proxy2, opts), expected2);
 assert.strictEqual(util.inspect(proxy3, opts), expected3);