From 86daa71190e68f44b64c92f2bc38b101a8de0f7f Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 29 Apr 2016 01:06:48 -0700 Subject: [PATCH 1/6] 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. Also adds util.isProxy() Fixes: https://github.com/nodejs/node/issues/6464 --- doc/api/util.md | 16 +++++++++++++ lib/util.js | 11 +++++++++ src/env.h | 2 ++ src/node_util.cc | 18 +++++++++++++++ test/parallel/test-util-inspect-proxy.js | 29 ++++++++++++++++++++++++ 5 files changed, 76 insertions(+) create mode 100644 test/parallel/test-util-inspect-proxy.js diff --git a/doc/api/util.md b/doc/api/util.md index d298ae4b44425a..67caab98368df6 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -478,6 +478,22 @@ util.isPrimitive(new Date()) // false ``` +## util.isProxy(object) + +Returns `true` if the given "object" is a `Proxy` object. Otherwise, returns +`false`. + +```js +const util = require('util'); +const proxyObj = new Proxy({}, {get: () => { return 5; }}); + +util.isProxy(proxyObj); + // true + +util.isProxy({}); + // false +``` + ## util.isRegExp(object) Stability: 0 - Deprecated diff --git a/lib/util.js b/lib/util.js index 63d6d0f3c865af..b8d4c8770b5212 100644 --- a/lib/util.js +++ b/lib/util.js @@ -241,6 +241,13 @@ function inspectPromise(p) { function formatValue(ctx, value, recurseTimes) { + + if (isProxy(value)) { + return 'Proxy ' + formatValue(ctx, + binding.getProxyDetails(value), + 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,6 +792,10 @@ exports.isPrimitive = isPrimitive; exports.isBuffer = Buffer.isBuffer; +function isProxy(p) { + return binding.isProxy(p); +} +exports.isProxy = isProxy; function pad(n) { return n < 10 ? '0' + n.toString(10) : n.toString(10); diff --git a/src/env.h b/src/env.h index afbade5dd81e70..fe0c636549eb6d 100644 --- a/src/env.h +++ b/src/env.h @@ -121,6 +121,7 @@ namespace node { V(fsevent_string, "FSEvent") \ V(gid_string, "gid") \ V(handle_string, "handle") \ + V(handler_string, "handler") \ V(heap_total_string, "heapTotal") \ V(heap_used_string, "heapUsed") \ V(homedir_string, "homedir") \ @@ -222,6 +223,7 @@ namespace node { V(subjectaltname_string, "subjectaltname") \ V(sys_string, "sys") \ V(syscall_string, "syscall") \ + V(target_string, "target") \ V(tick_callback_string, "_tickCallback") \ V(tick_domain_cb_string, "_tickDomainCallback") \ V(ticketkeycallback_string, "onticketkeycallback") \ diff --git a/src/node_util.cc b/src/node_util.cc index 1c5a2fa9a1f373..490b4a4f74c366 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -11,6 +11,7 @@ using v8::FunctionCallbackInfo; using v8::Local; using v8::Object; using v8::Private; +using v8::Proxy; using v8::String; using v8::Value; @@ -22,6 +23,7 @@ using v8::Value; V(isMap, IsMap) \ V(isMapIterator, IsMapIterator) \ V(isPromise, IsPromise) \ + V(isProxy, IsProxy) \ V(isRegExp, IsRegExp) \ V(isSet, IsSet) \ V(isSetIterator, IsSetIterator) \ @@ -37,6 +39,21 @@ using v8::Value; VALUE_METHOD_MAP(V) #undef V +static void GetProxyDetails(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + // Return nothing if it's not a proxy. + if (!args[0]->IsProxy()) + return; + + Local proxy = args[0].As(); + + Local ret = Object::New(env->isolate()); + ret->Set(env->handler_string(), proxy->GetHandler()); + ret->Set(env->target_string(), proxy->GetTarget()); + + args.GetReturnValue().Set(ret); +} static void GetHiddenValue(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -84,6 +101,7 @@ void Initialize(Local target, env->SetMethod(target, "getHiddenValue", GetHiddenValue); env->SetMethod(target, "setHiddenValue", SetHiddenValue); + env->SetMethod(target, "getProxyDetails", GetProxyDetails); } } // namespace util diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js new file mode 100644 index 00000000000000..6448df90c2a60f --- /dev/null +++ b/test/parallel/test-util-inspect-proxy.js @@ -0,0 +1,29 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const util = require('util'); +const processUtil = process.binding('util'); + +const target = {}; +const handler = { + get: function() { throw new Error('Getter should not be called'); } +}; +const proxyObj = new Proxy(target, handler); + +assert.strictEqual(util.isProxy(proxyObj), true); + +// Inspecting the proxy should not actually walk it's properties +assert.doesNotThrow(() => util.inspect(proxyObj)); + +const details = processUtil.getProxyDetails(proxyObj); +assert.deepStrictEqual(target, details.target); +assert.deepStrictEqual(handler, details.handler); + +assert.strictEqual(util.inspect(proxyObj), + 'Proxy { handler: { get: [Function] }, target: {} }'); + +// Using getProxyDetails with non-proxy returns undefined +assert.strictEqual(processUtil.getProxyDetails({}), undefined); +// isProxy with non-Proxy returns false +assert.strictEqual(util.isProxy({}), false); From ec16e916dfb21340e17a74d1b9246f503e47bffb Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 29 Apr 2016 19:11:32 -0700 Subject: [PATCH 2/6] Address nits --- doc/api/util.md | 16 ---------------- lib/util.js | 7 +------ test/parallel/test-util-inspect-proxy.js | 6 ++---- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 67caab98368df6..d298ae4b44425a 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -478,22 +478,6 @@ util.isPrimitive(new Date()) // false ``` -## util.isProxy(object) - -Returns `true` if the given "object" is a `Proxy` object. Otherwise, returns -`false`. - -```js -const util = require('util'); -const proxyObj = new Proxy({}, {get: () => { return 5; }}); - -util.isProxy(proxyObj); - // true - -util.isProxy({}); - // false -``` - ## util.isRegExp(object) Stability: 0 - Deprecated diff --git a/lib/util.js b/lib/util.js index b8d4c8770b5212..c0f5a9f96a0503 100644 --- a/lib/util.js +++ b/lib/util.js @@ -242,7 +242,7 @@ function inspectPromise(p) { function formatValue(ctx, value, recurseTimes) { - if (isProxy(value)) { + if (binding.isProxy(value)) { return 'Proxy ' + formatValue(ctx, binding.getProxyDetails(value), recurseTimes); @@ -792,11 +792,6 @@ exports.isPrimitive = isPrimitive; exports.isBuffer = Buffer.isBuffer; -function isProxy(p) { - return binding.isProxy(p); -} -exports.isProxy = isProxy; - function pad(n) { return n < 10 ? '0' + n.toString(10) : n.toString(10); } diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index 6448df90c2a60f..7d9996de931643 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -11,11 +11,11 @@ const handler = { }; const proxyObj = new Proxy(target, handler); -assert.strictEqual(util.isProxy(proxyObj), true); - // Inspecting the proxy should not actually walk it's properties assert.doesNotThrow(() => util.inspect(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); assert.deepStrictEqual(target, details.target); assert.deepStrictEqual(handler, details.handler); @@ -25,5 +25,3 @@ assert.strictEqual(util.inspect(proxyObj), // Using getProxyDetails with non-proxy returns undefined assert.strictEqual(processUtil.getProxyDetails({}), undefined); -// isProxy with non-Proxy returns false -assert.strictEqual(util.isProxy({}), false); From b2ebb7b4b2451b8ce3679730c3479f99a0a1ba7e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 29 Apr 2016 19:24:00 -0700 Subject: [PATCH 3/6] Cache the results to avoid duplicate lookups, add option --- benchmark/util/inspect-proxy.js | 44 ++++++++++++++++++++++++ doc/api/util.md | 3 ++ lib/util.js | 20 ++++++++--- test/parallel/test-util-inspect-proxy.js | 11 ++++-- 4 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 benchmark/util/inspect-proxy.js diff --git a/benchmark/util/inspect-proxy.js b/benchmark/util/inspect-proxy.js new file mode 100644 index 00000000000000..92d5a9a47d6dcb --- /dev/null +++ b/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'); + } +} diff --git a/doc/api/util.md b/doc/api/util.md index d298ae4b44425a..9f0979abe22706 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -179,6 +179,9 @@ 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 that are Proxy objects will be + introspected to show their `target` and `hander` objects. + Example of inspecting all properties of the `util` object: ```js diff --git a/lib/util.js b/lib/util.js index c0f5a9f96a0503..e866a408feee6a 100644 --- a/lib/util.js +++ b/lib/util.js @@ -122,6 +122,7 @@ function inspect(obj, opts) { // default options var ctx = { seen: [], + proxyCache: new WeakMap(), stylize: stylizeNoColor }; // legacy... @@ -139,6 +140,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); } @@ -242,10 +244,20 @@ function inspectPromise(p) { function formatValue(ctx, value, recurseTimes) { - if (binding.isProxy(value)) { - return 'Proxy ' + formatValue(ctx, - binding.getProxyDetails(value), - recurseTimes); + if (ctx.showProxy && typeof value === 'object') { + const inProxyCache = ctx.proxyCache.has(value); + var proxy = inProxyCache ? ctx.proxyCache.get(value) : undefined; + if (inProxyCache && proxy) { + return 'Proxy ' + formatValue(ctx, proxy, recurseTimes); + } else if (!inProxyCache) { + if (binding.isProxy(value)) { + proxy = binding.getProxyDetails(value); + ctx.proxyCache.set(value, proxy); + return 'Proxy ' + formatValue(ctx, proxy, recurseTimes); + } else { + ctx.proxyCache.set(value, undefined); + } + } } // Provide a hook for user-specified inspect functions. diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index 7d9996de931643..c612e789f90538 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -12,7 +12,7 @@ const handler = { const proxyObj = new Proxy(target, handler); // Inspecting the proxy should not actually walk it's properties -assert.doesNotThrow(() => util.inspect(proxyObj)); +assert.doesNotThrow(() => util.inspect(proxyObj, {showProxy: true})); // getProxyDetails is an internal method, not intended for public use. // This is here to test that the internals are working correctly. @@ -20,8 +20,15 @@ const details = processUtil.getProxyDetails(proxyObj); assert.deepStrictEqual(target, details.target); assert.deepStrictEqual(handler, details.handler); -assert.strictEqual(util.inspect(proxyObj), +assert.strictEqual(util.inspect(proxyObj, {showProxy: true}), 'Proxy { handler: { get: [Function] }, target: {} }'); // 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) +); From f630602059345d2aa9e46a0752c0b569d8985f36 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 30 Apr 2016 08:32:23 -0700 Subject: [PATCH 4/6] Reworked based on @bnoordhuis' feedback Just eliminated the isProxy call entirely, just try to get the proxy details and if it returns undefined, we know. Still have to cache the results either way to avoid duplicate lookup later. Simplified the native side to return an array as suggested. Expanded the test cases a bit more. It's still dog slow so the showProxy option has to be enabled for it to walk the path. Also, since apparently functions can also be proxied, check for that too. --- lib/util.js | 43 ++++++++++++----- src/env.h | 2 - src/node_util.cc | 10 ++-- test/parallel/test-util-inspect-proxy.js | 60 ++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 25 deletions(-) diff --git a/lib/util.js b/lib/util.js index e866a408feee6a..d3094850ca7228 100644 --- a/lib/util.js +++ b/lib/util.js @@ -122,7 +122,6 @@ function inspect(obj, opts) { // default options var ctx = { seen: [], - proxyCache: new WeakMap(), stylize: stylizeNoColor }; // legacy... @@ -244,19 +243,37 @@ function inspectPromise(p) { function formatValue(ctx, value, recurseTimes) { - if (ctx.showProxy && typeof value === 'object') { - const inProxyCache = ctx.proxyCache.has(value); - var proxy = inProxyCache ? ctx.proxyCache.get(value) : undefined; - if (inProxyCache && proxy) { - return 'Proxy ' + formatValue(ctx, proxy, recurseTimes); - } else if (!inProxyCache) { - if (binding.isProxy(value)) { - proxy = binding.getProxyDetails(value); - ctx.proxyCache.set(value, proxy); - return 'Proxy ' + formatValue(ctx, proxy, recurseTimes); - } else { - ctx.proxyCache.set(value, undefined); + if (ctx.showProxy && + (typeof value === 'object' || typeof value === 'function')) { + var proxy = undefined; + var proxyCache = ctx.proxyCache; + if (!proxyCache) + proxyCache = ctx.proxyCache = new WeakMap(); + // 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. + 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); } } diff --git a/src/env.h b/src/env.h index fe0c636549eb6d..afbade5dd81e70 100644 --- a/src/env.h +++ b/src/env.h @@ -121,7 +121,6 @@ namespace node { V(fsevent_string, "FSEvent") \ V(gid_string, "gid") \ V(handle_string, "handle") \ - V(handler_string, "handler") \ V(heap_total_string, "heapTotal") \ V(heap_used_string, "heapUsed") \ V(homedir_string, "homedir") \ @@ -223,7 +222,6 @@ namespace node { V(subjectaltname_string, "subjectaltname") \ V(sys_string, "sys") \ V(syscall_string, "syscall") \ - V(target_string, "target") \ V(tick_callback_string, "_tickCallback") \ V(tick_domain_cb_string, "_tickDomainCallback") \ V(ticketkeycallback_string, "onticketkeycallback") \ diff --git a/src/node_util.cc b/src/node_util.cc index 490b4a4f74c366..4465a3a8a7359a 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -6,6 +6,7 @@ namespace node { namespace util { +using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; using v8::Local; @@ -23,7 +24,6 @@ using v8::Value; V(isMap, IsMap) \ V(isMapIterator, IsMapIterator) \ V(isPromise, IsPromise) \ - V(isProxy, IsProxy) \ V(isRegExp, IsRegExp) \ V(isSet, IsSet) \ V(isSetIterator, IsSetIterator) \ @@ -42,15 +42,15 @@ using v8::Value; static void GetProxyDetails(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - // Return nothing if it's not a proxy. + // Return undefined if it's not a proxy. if (!args[0]->IsProxy()) return; Local proxy = args[0].As(); - Local ret = Object::New(env->isolate()); - ret->Set(env->handler_string(), proxy->GetHandler()); - ret->Set(env->target_string(), proxy->GetTarget()); + Local ret = Array::New(env->isolate(), 2); + ret->Set(0, proxy->GetTarget()); + ret->Set(1, proxy->GetHandler()); args.GetReturnValue().Set(ret); } diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index c612e789f90538..769113efdf279e 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -4,6 +4,7 @@ require('../common'); const assert = require('assert'); const util = require('util'); const processUtil = process.binding('util'); +const opts = {showProxy: true}; const target = {}; const handler = { @@ -12,16 +13,16 @@ const handler = { const proxyObj = new Proxy(target, handler); // Inspecting the proxy should not actually walk it's properties -assert.doesNotThrow(() => util.inspect(proxyObj, {showProxy: true})); +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.deepStrictEqual(target, details.target); -assert.deepStrictEqual(handler, details.handler); +assert.deepStrictEqual(target, details[0]); +assert.deepStrictEqual(handler, details[1]); -assert.strictEqual(util.inspect(proxyObj, {showProxy: true}), - 'Proxy { handler: { get: [Function] }, target: {} }'); +assert.strictEqual(util.inspect(proxyObj, opts), + 'Proxy [ {}, { get: [Function] } ]'); // Using getProxyDetails with non-proxy returns undefined assert.strictEqual(processUtil.getProxyDetails({}), undefined); @@ -32,3 +33,52 @@ assert.strictEqual(processUtil.getProxyDetails({}), undefined); 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]'); From 6115df50293ae00db72545912f011cd9aec76a04 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 30 Apr 2016 09:18:01 -0700 Subject: [PATCH 5/6] Fix doc nit --- doc/api/util.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 9f0979abe22706..fc1d2ba6609c70 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -179,8 +179,9 @@ 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 that are Proxy objects will be - introspected to show their `target` and `hander` objects. + - `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: From 842a8e4275b86ab5691b56bac6669adca116c741 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 1 May 2016 06:41:54 -0700 Subject: [PATCH 6/6] Address nits --- lib/util.js | 11 ++++++++--- src/node_util.cc | 4 +--- test/parallel/test-util-inspect-proxy.js | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/util.js b/lib/util.js index d3094850ca7228..69df2501997309 100644 --- a/lib/util.js +++ b/lib/util.js @@ -242,13 +242,13 @@ function inspectPromise(p) { function formatValue(ctx, value, recurseTimes) { - if (ctx.showProxy && - (typeof value === 'object' || typeof value === 'function')) { + ((typeof value === 'object' && value !== null) || + typeof value === 'function')) { var proxy = undefined; var proxyCache = ctx.proxyCache; if (!proxyCache) - proxyCache = ctx.proxyCache = new WeakMap(); + 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)) { @@ -264,6 +264,11 @@ function formatValue(ctx, value, recurseTimes) { 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. diff --git a/src/node_util.cc b/src/node_util.cc index 4465a3a8a7359a..9fc94acf094426 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -40,15 +40,13 @@ using v8::Value; #undef V static void GetProxyDetails(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - // Return undefined if it's not a proxy. if (!args[0]->IsProxy()) return; Local proxy = args[0].As(); - Local ret = Array::New(env->isolate(), 2); + Local ret = Array::New(args.GetIsolate(), 2); ret->Set(0, proxy->GetTarget()); ret->Set(1, proxy->GetHandler()); diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index 769113efdf279e..6311884b85b5f1 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -18,8 +18,8 @@ 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.deepStrictEqual(target, details[0]); -assert.deepStrictEqual(handler, details[1]); +assert.strictEqual(target, details[0]); +assert.strictEqual(handler, details[1]); assert.strictEqual(util.inspect(proxyObj, opts), 'Proxy [ {}, { get: [Function] } ]');