Node: `console.log(proxy)` invokes `get` trap with built-in Symbols

[zbjornson]

• Version: v6.9.2 and v7.4.0
• Platform: Ubuntu 16.06 LTS
• Subsystem: console, util

Calling console.log (util.inspect with default opts) on an instance of a Proxy invokes the get trap with some built-in Symbols. (It also invokes the trap with "inspect" and "valueOf", which is different than browser behavior but doesn't seem as problematic.)

In node:

> var p = new Proxy({a: 1}, {get(o, p) { console.log(typeof p, p); }}); console.log(p)
symbol Symbol(util.inspect.custom)
string inspect
string valueOf
symbol Symbol(Symbol.toStringTag)
{ a: 1 }

In chrome:

> var p = new Proxy({a: 1}, {get(o, p) { console.log(typeof p, p); }}); console.log(p)
Proxy {a: 1}

In FF:

> var p = new Proxy({a: 1}, {get(o, p) { console.log(typeof p, p); }}); console.log(p)
Proxy { <target>: Object, <handler>: Object }

In Edge:

MSEdge output

var p = new Proxy({a: 1}, {get(o, p) { console.log(typeof p, p); }}); console.log(p)
undefined
string toString
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string __defineGetter__
eval code (7) (1,40)
string __defineSetter__
eval code (7) (1,40)
string __lookupGetter__
eval code (7) (1,40)
string __lookupSetter__
eval code (7) (1,40)
string __proto__
eval code (7) (1,40)
string a
eval code (7) (1,40)
string constructor
eval code (7) (1,40)
string hasOwnProperty
eval code (7) (1,40)
string isPrototypeOf
eval code (7) (1,40)
string propertyIsEnumerable
eval code (7) (1,40)
string toLocaleString
eval code (7) (1,40)
string toString
eval code (7) (1,40)
string valueOf
eval code (7) (1,40)
string toString
eval code (7) (1,40)
[object Object]
eval code (7) (1,71)
   {
      __defineGetter__: undefined,
      __defineSetter__: undefined,
      __lookupGetter__: undefined,
      __lookupSetter__: undefined,
      __proto__: undefined,
      a: undefined,
      constructor: undefined,
      hasOwnProperty: undefined,
      isPrototypeOf: undefined,
      propertyIsEnumerable: undefined,
      toLocaleString: undefined,
      toString: undefined,
      valueOf: undefined
   }

If you're expecting your object to have Symbol keys and thus handle them, that's fine, but if you're only using string keys, then this can make it necessary to add a guard clause like if (typeof p !== "string") return o[p]; within the get trap. None of the above three browsers invoke the trap with a Symbol, by comparison.

The showProxy: true option for util.inspect prevents these four calls and making it the default could be a fix, but from the comments in #6465 it is apparently too slow. However, a simple isProxy binding (args.GetReturnValue().Set(args[0]->IsProxy())) is 10x faster than the getProxyDetails binding (29 ms vs 280 ms for 1M ops), and the target can be inspected the same as a plain object (without the binding). It seems like there's some special casing that could be done cheaply to avoid invoking the get trap with symbols, or even invoking it at all.

Maybe this is a wontfix -- maybe it's only sane if symbols are handled -- but right now this seems like an addition required to make some get traps compatible with node.

[jasnell]

I have long been wanting to revisit the proxy inspection code to see if we can improve performance. If you'd like to look into it and come up with a PR I'd be happy to help!

[zbjornson]

@jasnell cool. Happy to. Thoughts on desired behavior?

• <target> (node's default behavior), with the trap invocations avoided
• Proxy [<target> <handler>] (node's behavior with showProxy: true), if we can get it fast enough
• Proxy <target> (Chrome's behavior)

[jasnell]

I tend to prefer Chrome's behavior if we can get it fast enough. The key challenge is that we end up having to do the isProxy check on every value and on some keys (e.g. when inspecting things like Map), which is what leads to the significant performance hit.

[jasnell]

Also keep in mind the original motivation for the showProxy change -- that is, to avoid other troublesome issues and side effects that occur with Proxy traps (get in particular)

[simonkcleung]

var o={valueOf:function(){console.log("X");return 1}}
console.log(o);

{ [Number: 1] valueOf: [Function: valueOf] }

Not only Proxy but also normal Object.

[Trott]

Anyone still working on this? Maybe it's time to add a help wanted label?

[zbjornson]

I haven't had time to keep working on this, no. :(

[Kondax]

Any possible workarounds to this?
Nevermind, just returning the object itself (when the get trap is triggered by a symbol) seems to do.

[janl]

possible workaround here: https://stackoverflow.com/a/46928371/242298

[BridgeAR]

On master it will only trigger symbol calls such as:

symbol Symbol(util.inspect.custom)
symbol Symbol(Symbol.toStringTag)
symbol Symbol(Symbol.iterator)

It would be possible to work around this specific trap but then we end up in a different one. I know this is not ideal but I do not really see a way we can address this ideally. If anyone has some ideas, please leave a comment!

[bathos]

I’d like to make a case for not giving Proxies special treatment. In fact I think the showProxy option should be removed entirely, but assuming that’s not an option, at the least it should not be defaulting to true in any context (like it does in the REPL) and shouldn’t be doing any additional special casing.

I understand that proxies are perceived as having a higher risk of having buggy implementations. That’s probably true; no matter how well you understand the invariants of the internal methods being trapped, it’s still pretty easy to make mistakes. It’s a low-level (from an ES perspective) API. But while there will be flawed implementations of internal methods like [[Get]], this isn’t really different from there already being accessors that throw in the wild, and since [[Get]] is the biggie, this can likely be mitigated by doing a [[Has]] check first (e.g. Symbol.toStringTag in object), which doesn’t need to be specific to proxies.

Proxies are one of the few concepts in ES which can be categorically called an implementation detail. Other “implementation details” are mostly observable, but proxies are not. Both a target and a handler object may obtain the same level of privacy as a closure/scope, and no code with access to a given object it did not create can know if it is a proxy or not, by design.

The presentation of objects _as_ proxies in the REPL is mysterious. Even [util.inspect.custom] is ignored and Node prints details of the internal implementation instead of the realized public interface. Since it’s not uncommon for the target of a proxy to have no own enumerable properties, it often will just print Proxy [ {}, { defineProperty: [Function: defineProperty], ...etc } ]. This presentation is particularly frustrating when proxy objects are used in a public API that should be learnable by exploring from the REPL or with logging. Nothing shown relates to the object’s interface as seen by other code.

But in addition to seeming at odds with the intentions of Proxy objects, the mechanics of observing Proxy implementations in util.inspect are themselves problematic. It doesn’t just observe that a given object _is_ a proxy exotic object — it actually obtains references to the target and handler, values which haven’t been made available to that scope. The V8 C++ API calls that are necessary for achieving the current behavior violate invariants of ECMAScript itself. If the values never escaped util, that might be okay, but presently, it actually is exploitable:

const util = require('util');

const victim = function() {
  return new Proxy({}, {
    SECRET: 'One of the few guarantees in JS: nothing outside can see this'
  });
}();

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = value => {
      secret = value;
      options.stylize = stylize;
      return '';
    }

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret);

This exploit can be fixed pretty easily. But I think it’s symptomatic. My understanding was that V8’s C++ APIs are intended for making external functionality available to ECMAScript using ECMAScript values and ECMAScript semantics. Here though, it is being used to _bypass_ ECMAScript semantics. The behavior seen in the above script isn’t possible in ECMAScript — but neither is distinguishing between objects which have been implemented as ordinary objects and objects implemented as proxy exotic objects.

[BridgeAR]

To get back to the original issue: the reason why the traps are not called in the browser is that they use internals that Node.js does (and I would say _should_) not use. We mainly use regular JavaScript to access all the information. That way we have to trigger some of the traps. We could switch triggering one trap for another but that should not really be helpful.

We also trigger the ownKeys, getPrototypeOf and the getOwnPropertyDescriptor traps since we have calls that trigger those traps. I don't really think we can change much about this. If I am mistaken, please give me a hint, otherwise I would like to go ahead and close this issue as a won't fix.

---

@simonkcleung this is not triggered in newer Node.js versions anymore.

@bathos I have no strong opinion about the way proxy inspection currently works but to me it seems like a separate concern from the issue that proxy traps are invoked during inspection. Would you be so kind and open a separate issue about this?

[zbjornson]

I won't be hurt at all if this is closed as wont-fix. It makes sense that proxies should be written to handle various inputs safely (at least when they're publicly accessible). I think the original point remains valid, though, that Node.js' inspect functionality could avoid the issue. It might not be worth fixing (it sounds difficult), but it seems like it'll remain a gotcha for Node.js users.

[BridgeAR]

If anyone stumbles upon a good idea how to prevent some traps, please leave a comment.

[BridgeAR]

I have a PR open to fix proxy inspection: #26241

With that PR no proxy trap is going to be triggered anymore.

[tylerlong]

With that PR no proxy trap is going to be triggered anymore.

What if I need a trap to know that an object is inspected? If proxy doesn't support it, then the trap has to be installed onto the target object directly, which is not good.