10000 util: fix inspecting of proxy objects · nodejs/node@0bca959 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0bca959

Browse files
jasnellFishrock123
jasnell
authored and
Fishrock123
committed
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: #6464 PR-URL: #6465 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent d674493 commit 0bca959

File tree

5 files changed

+189
-1
lines changed

5 files changed

+189
-1
lines changed

‎benchmark/util/inspect-proxy.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
'use strict';
2+
3+
const util = require('util');
4+
const common = require('../common.js');
5+
6+
const bench = common.createBenchmark(main, {
7+
v: [1, 2],
8+
n: [1e6]
9+
});
10+
11+
function twoDifferentProxies(n) {
12+
// This one should be slower between we're looking up multiple proxies.
13+
const proxyA = new Proxy({}, {get: () => {}});
14+
const proxyB = new Proxy({}, {get: () => {}});
15+
bench.start();
16+
for (var i = 0; i < n; i += 1)
17+
util.inspect({a: proxyA, b: proxyB}, {showProxy: true});
18+
bench.end(n);
19+
}
20+
21+
function oneProxy(n) {
22+
// This one should be a bit faster because of the internal caching.
23+
const proxy = new Proxy({}, {get: () => {}});
24+
bench.start();
25+
for (var i = 0; i < n; i += 1)
26+
util.inspect({a: proxy, b: proxy}, {showProxy: true});
27+
bench.end(n);
28+
}
29+
30+
function main(conf) {
31+
const n = conf.n | 0;
32+
const v = conf.v | 0;
33+
34+
switch (v) {
35+
case 1:
36+
oneProxy(n);
37+
break;
38+
case 2:
39+
twoDifferentProxies(n);
40+
break;
41+
default:
42+
throw new Error('Should not get to here');
43+
}
44+
}

‎doc/api/util.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,10 @@ formatted string:
179179
- `customInspect` - if `false`, then custom `inspect(depth, opts)` functions
180180
defined on the objects being inspected won't be called. Defaults to `true`.
181181

182+
- `showProxy` - if `true`, then objects and functions that are Proxy objects
183+
will be introspected to show their `target` and `hander` objects. Defaults to
184+
`false`.
185+
182186
Example of inspecting all properties of the `util` object:
183187

184188
```js

‎lib/util.js‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ function inspect(obj, opts) {
139139
if (ctx.depth === undefined) ctx.depth = 2;
140140
if (ctx.colors === undefined) ctx.colors = false;
141141
if (ctx.customInspect === undefined) ctx.customInspect = true;
142+
if (ctx.showProxy === undefined) ctx.showProxy = false;
142143
if (ctx.colors) ctx.stylize = stylizeWithColor;
143144
return formatValue(ctx, obj, ctx.depth);
144145
}
@@ -241,6 +242,46 @@ function inspectPromise(p) {
241242

242243

243244
function formatValue(ctx, value, recurseTimes) {
245+
if (ctx.showProxy &&
246+
((typeof value === 'object' && value !== null) ||
247+
typeof value === 'function')) {
248+
var proxy = undefined;
249+
var proxyCache = ctx.proxyCache;
250+
if (!proxyCache)
251+
proxyCache = ctx.proxyCache = new Map();
252+
// Determine if we've already seen this object and have
253+
// determined that it either is or is not a proxy.
254+
if (proxyCache.has(value)) {
255+
// We've seen it, if the value is not undefined, it's a Proxy.
256+
proxy = proxyCache.get(value);
257+
} else {
258+
// Haven't seen it. Need to check.
259+
// If it's not a Proxy, this will return undefined.
260+
// Otherwise, it'll return an array. The first item
261+
// is the target, the second item is the handler.
262+
// We ignore (and do not return) the Proxy isRevoked property.
263+
proxy = binding.getProxyDetails(value);
264+
if (proxy) {
265+
// We know for a fact that this isn't a Proxy.
266+
// Mark it as having already been evaluated.
267+
// We do this because this object is passed
268+
// recursively to formatValue below in order
269+
// for it to get proper formatting, and because
270+
// the target and handle objects also might be
271+
// proxies... it's unfortunate but necessary.
272+
proxyCache.set(proxy, undefined);
273+
}
274+
// If the object is not a Proxy, then this stores undefined.
275+
// This tells the code above that we've already checked and
276+
// ruled it out. If the object is a proxy, this caches the
277+
// results of the getProxyDetails call.
278+
proxyCache.set(value, proxy);
279+
}
280+
if (proxy) {
281+
return 'Proxy ' + formatValue(ctx, proxy, recurseTimes);
282+
}
283+
}
284+
244285
// Provide a hook for user-specified inspect functions.
245286
// Check that value is an object with an inspect function on it
246287
if (ctx.customInspect &&
@@ -785,7 +826,6 @@ exports.isPrimitive = isPrimitive;
785826

786827
exports.isBuffer = Buffer.isBuffer;
787828

788-
789829
function pad(n) {
790830
return n < 10 ? '0' + n.toString(10) : n.toString(10);
791831
}

‎src/node_util.cc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
namespace node {
77
namespace util {
88

9+
using v8::Array;
910
using v8::Context;
1011
using v8::FunctionCallbackInfo;
1112
using v8::Local;
1213
using v8::Object;
1314
using v8::Private;
15+
using v8::Proxy;
1416
using v8::String;
1517
using v8::Value;
1618

@@ -37,6 +39,19 @@ using v8::Value;
3739
VALUE_METHOD_MAP(V)
3840
#undef V
3941

42+
static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
43+
// Return undefined if it's not a proxy.
44+
if (!args[0]->IsProxy())
45+
return;
46+
47+
Local<Proxy> proxy = args[0].As<Proxy>();
48+
49+
Local<Array> ret = Array::New(args.GetIsolate(), 2);
50+
ret->Set(0, proxy->GetTarget());
51+
ret->Set(1, proxy->GetHandler());
52+
53+
args.GetReturnValue().Set(ret);
54+
}
4055

4156
static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
4257
Environment* env = Environment::GetCurrent(args);
@@ -84,6 +99,7 @@ void Initialize(Local<Object> target,
8499

85100
env->SetMethod(target, "getHiddenValue", GetHiddenValue);
86101
env->SetMethod(target, "setHiddenValue", SetHiddenValue);
102+
env->SetMethod(target, "getProxyDetails", GetProxyDetails);
87103
}
88104

89105
} // namespace util

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('assert');
5+
const util = require('util');
6+
const processUtil = process.binding('util');
7+
const opts = {showProxy: true};
8+
9+
const target = {};
10+
const handler = {
11+
get: function() { throw new Error('Getter should not be called'); }
12+
};
13+
const proxyObj = new Proxy(target, handler);
14+
15+
// Inspecting the proxy should not actually walk it's properties
16+
assert.doesNotThrow(() => util.inspect(proxyObj, opts));
17+
18+
// getProxyDetails is an internal method, not intended for public use.
19+
// This is here to test that the internals are working correctly.
20+
const details = processUtil.getProxyDetails(proxyObj);
21+
assert.strictEqual(target, details[0]);
22+
assert.strictEqual(handler, details[1]);
23+
24+
assert.strictEqual(util.inspect(proxyObj, opts),
25+
'Proxy [ {}, { get: [Function] } ]');
26+
27+
// Using getProxyDetails with non-proxy returns undefined
28+
assert.strictEqual(processUtil.getProxyDetails({}), undefined);
29+
30+
// This will throw because the showProxy option is not used
31+
// and the get function on the handler object defined above
32+
// is actually invoked.
33+
assert.throws(
34+
() => util.inspect(proxyObj)
35+
);
36+
37+
// Yo dawg, I heard you liked Proxy so I put a Proxy
38+
// inside your Proxy that proxies your Proxy's Proxy.
39+
const proxy1 = new Proxy({}, {});
40+
const proxy2 = new Proxy(proxy1, {});
41+
const proxy3 = new Proxy(proxy2, proxy1);
42+
const proxy4 = new Proxy(proxy1, proxy2);
43+
const proxy5 = new Proxy(proxy3, proxy4);
44+
const proxy6 = new Proxy(proxy5, proxy5);
45+
const expected0 = '{}';
46+
const expected1 = 'Proxy [ {}, {} ]';
47+
const expected2 = 'Proxy [ Proxy [ {}, {} ], {} ]';
48+
const expected3 = 'Proxy [ Proxy [ Proxy [ {}, {} ], {} ], Proxy [ {}, {} ] ]';
49+
const expected4 = 'Proxy [ Proxy [ {}, {} ], Proxy [ Proxy [ {}, {} ], {} ] ]';
50+
const expected5 = 'Proxy [ Proxy [ Proxy [ Proxy [Object], {} ],' +
51+
' Proxy [ {}, {} ] ],\n Proxy [ Proxy [ {}, {} ]' +
52+
', Proxy [ Proxy [Object], {} ] ] ]';
53+
const expected6 = 'Proxy [ Proxy [ Proxy [ Proxy [Object], Proxy [Object]' +
54+
' ],\n Proxy [ Proxy [Object], Proxy [Object] ] ],\n' +
55+
' Proxy [ Proxy [ Proxy [Object], Proxy [Object] ],\n' +
56+
' Proxy [ Proxy [Object], Proxy [Object] ] ] ]';
57+
assert.strictEqual(util.inspect(proxy1, opts), expected1);
58+
assert.strictEqual(util.inspect(proxy2, opts), expected2);
59+
assert.strictEqual(util.inspect(proxy3, opts), expected3);
60+
assert.strictEqual(util.inspect(proxy4, opts), expected4);
61+
assert.strictEqual(util.inspect(proxy5, opts), expected5);
62+
assert.strictEqual(util.inspect(proxy6, opts), expected6);
63+
assert.strictEqual(util.inspect(proxy1), expected0);
64+
assert.strictEqual(util.inspect(proxy2), expected0);
65+
assert.strictEqual(util.inspect(proxy3), expected0);
66+
assert.strictEqual(util.inspect(proxy4), expected0);
67+
assert.strictEqual(util.inspect(proxy5), expected0);
68+
assert.strictEqual(util.inspect(proxy6), expected0);
69+
70+
// Just for fun, let's create a Proxy using Arrays.
71+
const proxy7 = new Proxy([], []);
72+
const expected7 = 'Proxy [ [], [] ]';
73+
assert.strictEqual(util.inspect(proxy7, opts), expected7);
74+
assert.strictEqual(util.inspect(proxy7), '[]');
75+
76+
// Now we're just getting silly, right?
77+
const proxy8 = new Proxy(Date, []);
78+
const proxy9 = new Proxy(Date, String);
79+
const expected8 = 'Proxy [ [Function: Date], [] ]';
80+
const expected9 = 'Proxy [ [Function: Date], [Function: String] ]';
81+
assert.strictEqual(util.inspect(proxy8, opts), expected8);
82+
assert.strictEqual(util.inspect(proxy9, opts), expected9);
83+
assert.strictEqual(util.inspect(proxy8), '[Function: Date]');
84+
assert.strictEqual(util.inspect(proxy9), '[Function: Date]');

0 commit comments

Comments
 (0)
0