Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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: #6464
  • Loading branch information
jasnell committed Apr 29, 2016
commit 86daa71190e68f44b64c92f2bc38b101a8de0f7f
16 changes: 16 additions & 0 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -785,6 +792,10 @@ exports.isPrimitive = isPrimitive;

exports.isBuffer = Buffer.isBuffer;

function isProxy(p) {
return binding.isProxy(p);
}
exports.isProxy = isProxy;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you export this, it becomes public API, and should be documented.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 ... Forgot to add the doc for it. Will add soon!
On Apr 29, 2016 6:29 AM, "Colin Ihrig" notifications@github.com wrote:

In lib/util.js
#6465 (comment):

@@ -785,6 +792,10 @@ exports.isPrimitive = isPrimitive;

exports.isBuffer = Buffer.isBuffer;

+function isProxy(p) {

  • return binding.isProxy(p);
    +}
    +exports.isProxy = isProxy;

If you export this, it becomes public API, and should be documented.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/6465/files/3d4ff4311a3cf1ff756413756ffc859a663aba99#r61577439


function pad(n) {
return n < 10 ? '0' + n.toString(10) : n.toString(10);
Expand Down
2 changes: 2 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down Expand Up @@ -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") \
Expand Down
18 changes: 18 additions & 0 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) \
Expand All @@ -37,6 +39,21 @@ using v8::Value;
VALUE_METHOD_MAP(V)
#undef V

static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

// Return nothing if it's not a proxy.
if (!args[0]->IsProxy())
return;

Local<Proxy> proxy = args[0].As<Proxy>();

Local<Object> ret = Object::New(env->isolate());
ret->Set(env->handler_string(), proxy->GetHandler());
ret->Set(env->target_string(), proxy->GetTarget());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean::New(env->isolate(), proxy->IsRevoked()) might be of interest as well, although I guess you can also infer that by checking if the handler is non-null.

Aside, it might be a little cheaper to return an array instead of a dictionary object.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still (even) cheaper to pass an array/object from js?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, can do.


args.GetReturnValue().Set(ret);
}

static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand Down Expand Up @@ -84,6 +101,7 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "getHiddenValue", GetHiddenValue);
env->SetMethod(target, "setHiddenValue", SetHiddenValue);
env->SetMethod(target, "getProxyDetails", GetProxyDetails);
}

} // namespace util
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-util-inspect-proxy.js
Original file line number Diff line number Diff line change
@@ -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: {} }');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you line up the arguments?


// Using getProxyDetails with non-proxy returns undefined
assert.strictEqual(processUtil.getProxyDetails({}), undefined);
// isProxy with non-Proxy returns false
assert.strictEqual(util.isProxy({}), false);