util: fix inspecting of proxy objects

In certain conditions, inspecting a Proxy object can lead to a
max call stack error. Avoid that by detecting the Proxy object
and outputting information about the Proxy object itself.

Fixes: nodejs#6464
PR-URL: nodejs#6465
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

benchmark/util/inspect-proxy.js

@@ -0,0 +1,44 @@
+'use strict';
+
+const util = require('util');
+const common = require('../common.js');
+
+const bench = common.createBenchmark(main, {
+  v: [1, 2],
+  n: [1e6]
+});
+
+function twoDifferentProxies(n) {
+  // This one should be slower between we're looking up multiple proxies.
+  const proxyA = new Proxy({}, {get: () => {}});
+  const proxyB = new Proxy({}, {get: () => {}});
+  bench.start();
+  for (var i = 0; i < n; i += 1)
+    util.inspect({a: proxyA, b: proxyB}, {showProxy: true});
+  bench.end(n);
+}
+
+function oneProxy(n) {
+  // This one should be a bit faster because of the internal caching.
+  const proxy = new Proxy({}, {get: () => {}});
+  bench.start();
+  for (var i = 0; i < n; i += 1)
+    util.inspect({a: proxy, b: proxy}, {showProxy: true});
+  bench.end(n);
+}
+
+function main(conf) {
+  const n = conf.n | 0;
+  const v = conf.v | 0;
+
+  switch (v) {
+    case 1:
+      oneProxy(n);
+      break;
+    case 2:
+      twoDifferentProxies(n);
+      break;
+    default:
+      throw new Error('Should not get to here');
+  }
+}

doc/api/util.md

@@ -179,6 +179,10 @@ formatted string:
  - `customInspect` - if `false`, then custom `inspect(depth, opts)` functions
    defined on the objects being inspected won't be called. Defaults to `true`.
 
+ - `showProxy` - if `true`, then objects and functions that are Proxy objects
+   will be introspected to show their `target` and `hander` objects. Defaults to
+   `false`.
+
 Example of inspecting all properties of the `util` object:
 
 ```js

lib/util.js

@@ -139,6 +139,7 @@ function inspect(obj, opts) {
   if (ctx.depth === undefined) ctx.depth = 2;
   if (ctx.colors === undefined) ctx.colors = false;
   if (ctx.customInspect === undefined) ctx.customInspect = true;
+  if (ctx.showProxy === undefined) ctx.showProxy = false;
   if (ctx.colors) ctx.stylize = stylizeWithColor;
   return formatValue(ctx, obj, ctx.depth);
 }
@@ -241,6 +242,46 @@ function inspectPromise(p) {
 
 
 function formatValue(ctx, value, recurseTimes) {
+  if (ctx.showProxy &&
+      ((typeof value === 'object' && value !== null) ||
+       typeof value === 'function')) {
+    var proxy = undefined;
+    var proxyCache = ctx.proxyCache;
+    if (!proxyCache)
+      proxyCache = ctx.proxyCache = new Map();
+    // Determine if we've already seen this object and have
+    // determined that it either is or is not a proxy.
+    if (proxyCache.has(value)) {
+      // We've seen it, if the value is not undefined, it's a Proxy.
+      proxy = proxyCache.get(value);
+    } else {
+      // Haven't seen it. Need to check.
+      // If it's not a Proxy, this will return undefined.
+      // Otherwise, it'll return an array. The first item
+      // is the target, the second item is the handler.
+      // We ignore (and do not return) the Proxy isRevoked property.
+      proxy = binding.getProxyDetails(value);
+      if (proxy) {
+        // We know for a fact that this isn't a Proxy.
+        // Mark it as having already been evaluated.
+        // We do this because this object is passed
+        // recursively to formatValue below in order
+        // for it to get proper formatting, and because
+        // the target and handle objects also might be
+        // proxies... it's unfortunate but necessary.
+        proxyCache.set(proxy, undefined);
+      }
+      // If the object is not a Proxy, then this stores undefined.
+      // This tells the code above that we've already checked and
+      // ruled it out. If the object is a proxy, this caches the
+      // results of the getProxyDetails call.
+      proxyCache.set(value, proxy);
+    }
+    if (proxy) {
+      return 'Proxy ' + formatValue(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 &&
@@ -785,7 +826,6 @@ exports.isPrimitive = isPrimitive;
 
 exports.isBuffer = Buffer.isBuffer;
 
-
 function pad(n) {
   return n < 10 ? '0' + n.toString(10) : n.toString(10);
 }

src/node_util.cc

@@ -6,11 +6,13 @@
 namespace node {
 namespace util {
 
+using v8::Array;
 using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Local;
 using v8::Object;
 using v8::Private;
+using v8::Proxy;
 using v8::String;
 using v8::Value;
 
@@ -37,6 +39,19 @@ using v8::Value;
   VALUE_METHOD_MAP(V)
 #undef V
 
+static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
+  // Return undefined if it's not a proxy.
+  if (!args[0]->IsProxy())
+    return;
+
+  Local<Proxy> proxy = args[0].As<Proxy>();
+
+  Local<Array> ret = Array::New(args.GetIsolate(), 2);
+  ret->Set(0, proxy->GetTarget());
+  ret->Set(1, proxy->GetHandler());
+
+  args.GetReturnValue().Set(ret);
+}
 
 static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
@@ -84,6 +99,7 @@ void Initialize(Local<Object> target,
 
   env->SetMethod(target, "getHiddenValue", GetHiddenValue);
   env->SetMethod(target, "setHiddenValue", SetHiddenValue);
+  env->SetMethod(target, "getProxyDetails", GetProxyDetails);
 }
 
 }  // namespace util

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

@@ -0,0 +1,84 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const util = require('util');
+const processUtil = process.binding('util');
+const opts = {showProxy: true};
+
+const target = {};
+const handler = {
+  get: function() { throw new Error('Getter should not be called'); }
+};
+const proxyObj = new Proxy(target, handler);
+
+// Inspecting the proxy should not actually walk it's properties
+assert.doesNotThrow(() => util.inspect(proxyObj, opts));
+
+// 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);
+assert.strictEqual(target, details[0]);
+assert.strictEqual(handler, details[1]);
+
+assert.strictEqual(util.inspect(proxyObj, opts),
+                   'Proxy [ {}, { get: [Function] } ]');
+
+// 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)
+);
+
+// Yo dawg, I heard you liked Proxy so I put a Proxy
+// inside your Proxy that proxies your Proxy's Proxy.
+const proxy1 = new Proxy({}, {});
+const proxy2 = new Proxy(proxy1, {});
+const proxy3 = new Proxy(proxy2, proxy1);
+const proxy4 = new Proxy(proxy1, proxy2);
+const proxy5 = new Proxy(proxy3, proxy4);
+const proxy6 = new Proxy(proxy5, proxy5);
+const expected0 = '{}';
+const expected1 = 'Proxy [ {}, {} ]';
+const expected2 = 'Proxy [ Proxy [ {}, {} ], {} ]';
+const expected3 = 'Proxy [ Proxy [ Proxy [ {}, {} ], {} ], Proxy [ {}, {} ] ]';
+const expected4 = 'Proxy [ Proxy [ {}, {} ], Proxy [ Proxy [ {}, {} ], {} ] ]';
+const expected5 = 'Proxy [ Proxy [ Proxy [ Proxy [Object], {} ],' +
+                  ' Proxy [ {}, {} ] ],\n  Proxy [ Proxy [ {}, {} ]' +
+                  ', Proxy [ Proxy [Object], {} ] ] ]';
+const expected6 = 'Proxy [ Proxy [ Proxy [ Proxy [Object], Proxy [Object]' +
+                  ' ],\n    Proxy [ Proxy [Object], Proxy [Object] ] ],\n' +
+                  '  Proxy [ Proxy [ Proxy [Object], Proxy [Object] ],\n' +
+                  '    Proxy [ Proxy [Object], Proxy [Object] ] ] ]';
+assert.strictEqual(util.inspect(proxy1, opts), expected1);
+assert.strictEqual(util.inspect(proxy2, opts), expected2);
+assert.strictEqual(util.inspect(proxy3, opts), expected3);
+assert.strictEqual(util.inspect(proxy4, opts), expected4);
+assert.strictEqual(util.inspect(proxy5, opts), expected5);
+assert.strictEqual(util.inspect(proxy6, opts), expected6);
+assert.strictEqual(util.inspect(proxy1), expected0);
+assert.strictEqual(util.inspect(proxy2), expected0);
+assert.strictEqual(util.inspect(proxy3), expected0);
+assert.strictEqual(util.inspect(proxy4), expected0);
+assert.strictEqual(util.inspect(proxy5), expected0);
+assert.strictEqual(util.inspect(proxy6), expected0);
+
+// Just for fun, let's create a Proxy using Arrays.
+const proxy7 = new Proxy([], []);
+const expected7 = 'Proxy [ [], [] ]';
+assert.strictEqual(util.inspect(proxy7, opts), expected7);
+assert.strictEqual(util.inspect(proxy7), '[]');
+
+// Now we're just getting silly, right?
+const proxy8 = new Proxy(Date, []);
+const proxy9 = new Proxy(Date, String);
+const expected8 = 'Proxy [ [Function: Date], [] ]';
+const expected9 = 'Proxy [ [Function: Date], [Function: String] ]';
+assert.strictEqual(util.inspect(proxy8, opts), expected8);
+assert.strictEqual(util.inspect(proxy9, opts), expected9);
+assert.strictEqual(util.inspect(proxy8), '[Function: Date]');
+assert.strictEqual(util.inspect(proxy9), '[Function: Date]');