Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwrap proxies for native methods. #1114

Open
jdalton opened this issue Feb 22, 2018 · 38 comments
Open

Unwrap proxies for native methods. #1114

jdalton opened this issue Feb 22, 2018 · 38 comments

Comments

@jdalton
Copy link
Member

jdalton commented Feb 22, 2018

Many builtins require a this of a specific type and while proxied values quack like a duck and swim like a duck in many regards, like typeof, instanceof, isArray, etc., they fail at other points

Array.isArray(new Proxy([], {}))
// true

Object.prototype.toString.call(new Proxy([], {}))
// "[object Array]"

typeof new Proxy(function a() {}, {})
// "function"

// with the revised Function.prototype.toString spec
Function.prototype.toString.call(new Proxy(function a() {}, {}))
// "function() { [native code] }"

new Proxy(new Date, {}) instanceof Date
// true

But there are still gotchas like

new Proxy(new Set, {}).add(1)
// VM92:1 Uncaught TypeError: Method Set.prototype.add called on incompatible receiver
// [object Object]
//    at Proxy.add (<anonymous>)

It would be nice to smooth out the remaining gotchas with unwrapping or tweaking of checks.

@loganfsmyth
Copy link
Contributor

loganfsmyth commented Feb 23, 2018

Is adding more special cases specifically for native types desirable? With the private fields proposal, it'll become trivial for developers to add private fields to their own classes that exhibit these same types of issues, so this won't be an issue that only applies to native methods. Not only that, if they are already using a WeakMap branding approach, they'll have the same issues as this already.

If we accept that this is an edge case of proxy that needs solving (which I'm not totally set on), then it seems like a generalized solution would be important, otherwise you're just introducing most inconsistencies between native and non-native objects. (EDIT: And yeah, I don't see how a generalized solution could be achieved anyway).

@bakkot
Copy link
Contributor

bakkot commented Feb 23, 2018

Funnily enough, isArray and typeof actually work by very different mechanisms here: isArray actually peels apart proxies, whereas typeof works by the Proxy constructor copying over the [[Call]] internal slot, if it exists.

Anyway, even if we carefully added one or the other of those behaviors for all of the ES builtins, we wouldn't be in a position to add them for the web platform stuff: (new Proxy(new Image, {})).align // TypeError.

Personally I think the proxy-tunneling behavior of isArray was a bad idea in the first place, and should not be copied elsewhere. The easiest mental model is that proxies can only relied upon to forward the behavior which can be intercepted by a handler, and not for things like identity or branding.

At the very least, they definitely can't be relied upon for identity; new Proxy(a, handler) === a should never hold. But you can implement brand checks on top of identity, for example by keeping a WeakSet of everything your constructor creates, in which case there's no way a proxy can satisfy all possible brand checks. Better that we not try to fake it in an ad-hoc way.

@jdalton
Copy link
Member Author

jdalton commented Feb 23, 2018

A generalized solution is fine.

The inconsistency, as is, isn't great for devs. One of the useful bits of proxies is that they allow what they wrap to surface (typeof checks, instanceof, etc.). It'd be a shame to lose or lessen that.

@domenic
Copy link
Member

domenic commented Feb 23, 2018

I wish we could remove the isArray special-casing. Then it'd only be things that rely on an object's public properties (like instanceof or typeof) that are fakeable by proxies.

As-is, the mental model should be that proxies can proxy an object's public properties, plus if they're an array, there's a legacy mistake where they can proxy the array object's isArray-ness. Let's hold the line there, and not add more legacy mistakes.

@bakkot
Copy link
Contributor

bakkot commented Feb 23, 2018

There can't be a generalized solution. If I write

const images = [];
class Image {
  constructor(){
    images.push(this);
  }
  get align() {
    if (!images.includes(this)) throw new TypeError;
    return 'whatever';
  }
}

then

(new Image).align

works and

(new Proxy(new Image, {})).align

throws, and there is no change to the semantics of proxies we could reasonably make which would change that.

@jdalton
Copy link
Member Author

jdalton commented Feb 23, 2018

As is proxies have promise but end up being half useful.
Yes, they provide traps, but interacting with them is super fragile.
I'd dig a future where we have traps but also a more resilient object to interact with.

@erights
Copy link

erights commented Feb 23, 2018 via email

@jdalton
Copy link
Member Author

jdalton commented Feb 23, 2018

@erights Is there an example of a membrane implementation somewhere?

@erights
Copy link

erights commented Feb 23, 2018 via email

@erights
Copy link

erights commented Feb 23, 2018 via email

@erights
Copy link

erights commented Feb 23, 2018 via email

@erights
Copy link

erights commented Feb 23, 2018 via email

@jdalton
Copy link
Member Author

jdalton commented Feb 23, 2018

@erights has there been any movement, or a staged proposal, for membranes?

@domenic
Copy link
Member

domenic commented Feb 23, 2018

What public properties does typeof rely on?

The existence of [[Call]]. I admit to being imprecise in my usage of the word "property".

@erights
Copy link

erights commented Feb 23, 2018

But [[Call]] is an internal property, and thus in the same category as the internal properties that isArray relies on.

That said, I think I agree that we should have avoided adding more operations, like isArray, that rely on cross-Realm visibility of internal properties. I agree that we should avoid adding more.

@erights
Copy link

erights commented Feb 23, 2018

has there been any movement, or a staged proposal, for membranes?

Before standardizing, we should have more experience with user-written abstractions for creating membranes, like @ajvincent 's membrane library. So the answer is, indirectly yes. As these libraries get built and used, we make progress towards the experience needed to figure out what we should promote into platform-provided abstractions.

@ljharb
Copy link
Member

ljharb commented Feb 23, 2018

Re:

// with the revised Function.prototype.toString spec
Function.prototype.toString.call(new Proxy(function a() {}, {}))

I'm not sure that's true; I'm pretty certain that https://tc39.github.io/Function-prototype-toString-revision/#proposal-sec-function.prototype.tostring will still cause that example to throw, since none of the internal slots of functions are forwarded (edit: besides [[Call]])

@jdalton
Copy link
Member Author

jdalton commented Feb 23, 2018

@ljharb
I based that snippit on the result of this issue:
https://bugs.chromium.org/p/v8/issues/detail?id=7484

This used to be spec'd as a TypeError, but with the Function.prototype.toString revision, it seems we're supposed to generate something like

"function() { [native code] }"

This is step 3 of the new proposal:

 If Type(func) is Object and IsCallable(func) is true, then return an implementation-dependent
 String source code representation of func. The representation must have the syntax of a
 NativeFunction.

@domenic
Copy link
Member

domenic commented Feb 23, 2018

My reading of the revision spec is that it should return [native code], since it doesn't have a [[SourceText]] but IsCallable is true. And indeed, that's what V8 implements, at least per the tests: https://chromium.googlesource.com/v8/v8/+/f7d7b5c6a4a55baa8984525fba6d0d5e1355b3b0/test/mjsunit/harmony/function-tostring.js#126

@bakkot
Copy link
Contributor

bakkot commented Feb 23, 2018

Jordan, the proxy constructor copies over [[Call]] explicitly.

@erights
Copy link

erights commented Feb 23, 2018

@bakkot
Worth pointing out that
membrane(new Image()).align will work fine.

Brand checks across a membrane work fine. On the other side of the membrane, you are using a proxy for the brand checker.

The mistake with isArray is not that it tunnels through proxies, but rather that it reveals internal properties across a Realm boundary. Given that it does, it is good that it also does so, in a compatible way, across a membrane boundary.

@ljharb
Copy link
Member

ljharb commented Feb 23, 2018

Ah, ok - I do see that now. @michaelficarra, was that an intentional change in the revision?

@bakkot yes; the current toString text doesn't check [[Call]], it checks either a built-in function object or has an [[ECMAScriptCode]] internal slot - and I wasn't aware until just now that the proposal changes toString to allow for proxied functions to no longer be detected as proxied.

@bakkot
Copy link
Contributor

bakkot commented Feb 23, 2018

@erights

The mistake with isArray is not that it tunnels through proxies, but rather that it reveals internal properties across a Realm boundary. Given that it does, it is good that it also does so, in a compatible way, across a membrane boundary.

Tons of internal slot checks work across realms but not through proxies. For example, Map.prototype.has.call(mapFromOtherRealm) does not throw, even though Map.prototype.has.call(new Proxy(new Map, {})) does.

isArray really is aberrant in its behavior even beyond its "exposing things across realms" behavior.

@erights
Copy link

erights commented Feb 23, 2018

Notice that most of these, including your examples and most of the examples in this thread, are builtin methods on the prototype object that the relevant instances inherit from. Thus, the common form of lookup starts with the instance. This use works across membranes and across realms fine.

Granted that Foo.prototype.bar.call(aFooProxy) does work between realms (when Foo is a builtin) and not across a membrane. But this is unusual usage.

Array.isArray is indeed aberrant, because it is not looked up starting from the instances it uses, and it uses internal properties on these instances. Had we not made the special case, normal usage would have differed between realm boundaries and membrane boundaries.

@ajvincent
Copy link

For what it's worth, my membrane library does have a way of passing through a specified set of objects without wrapping them in proxies.

https://github.com/ajvincent/es-membrane/blob/master/spec/features/primordials.js#L40-L49

I've also included a bypass for primordials (Mark gave me the name for them) such as Object, Function, Array, etc. in the GUI configuration tool I've been building for my membrane library.

So, if you want these primordials unwrapped, my library does support that.

@littledan
Copy link
Member

If the semantics of x.y() on a proxy x were more like Reflect.get(x, "y", x.[[ProxyTarget]]).call(x.[[ProxyTarget]])), would that solve some of these problems of Proxy transparency? I guess this is what membrane libraries accomplish, it's just a bit difficult to do for your own code efficiently.

@erights
Copy link

erights commented Feb 23, 2018

Hi @littledan (attn @tvcutsem)
Indeed. For related but different reasons, we tried several times to enable a proxy's handler to distinguish a method call from a property lookup followed by a function call. None of those efforts succeeded, which I still regret. I do welcome efforts to try again, though they should be informed by the history of previous attempts and why they failed. Now that proxies have been shipping for quite a while, any attempts would also need to be adequately compatible to avoid breaking existing handler code.

Anyone have pointers to some of the discussions of those previous attempts?

Below, I reiterate my favorite proposal, which if it works would likely be sufficiently compatible to avoid breaking existing handler code. Note that my proposal below does not directly address your stated need, but it helps, and it does address other needs.

The current signature of the get trap is

handler.get(target, name, receiver)

Add to this a boolean argument,

handler.get(target, name, receiver, isMethodCall)

where a true is passed if the property is being gotten for purposes of being invoked immediately by a method call, where a method call has the following characteristics:

  • if caused by syntax, the syntax must have the form x.y(args) or x[e](args)
  • the following function call will use the receiver as its this argument
  • the following function call will not make the called function otherwise reachable.

This would raise a new problem: How would we enhance Reflect so that it could pass through this flag without violating the guarantees of the last two bullets above? I have thought of some kludges not worth repeating, but I have no clear answer to offer. Suggestions?

@erights
Copy link

erights commented Feb 23, 2018

To clarify, the guarantee provided by the last two bullets would enable a [[Get]] that reified the gotten method to reify a method bound to the receiver, while invisibly avoiding this allocation-and-binding step for a method call, that would provide the same this binding anyway.

@caridy
Copy link
Contributor

caridy commented Feb 23, 2018

@erights has there been any movement, or a staged proposal, for membranes?

@jdalton we have been discussing membranes a lot lately during the SES weekly meetings (in case you want to show up). As for proposing it as a new feature, I think most of the interest from the group is to provide the low-level APIs that can help you to build different types of membranes without too much hazard by using WeakMaps and Proxies. E.g.: very recently we built a light-weight membrane (<2k) inspired by @ajvincent's implementation that only allows distortion for outgoing values, but not for incoming ones, and that is proven to be sufficient for us to cover many cases.

@tvcutsem
Copy link

Back in 2012, we discussed adding a special "nonGenericCall" trap to the Proxy API to deal with proxies being passed as argument to built-in functions, even through 'static' method invocations. Example:

Date.prototype.getYear.call(new Proxy(tgt, handler))
// would be trapped as
handler.nonGenericCall(tgt, Date.prototype.getYear, [])

We decided against that. I don't remember what was the decisive argument, but I do remember @erights rightly pointing out that such a trap would give the proxy access to the function it was passed to, which is a new, previously unseen kind of data flow. The function could be a closely held capability, which the proxy could "steal" through the trap.

I also remember we discussed at that point that the above solution would not be adequate to handle classes with private state, so it would likely have been a half-baked solution anyway.

@tvcutsem
Copy link

tvcutsem commented Feb 25, 2018

As for proxies being able to distinguish property accesses from method invocations, I remember this was a perennial discussion point ever since we first proposed proxies.

What I remember as one decisive argument from past discussions is that the invariant that a method call in JS is really just property selection followed by function application is too fundamental to the language semantics, and proxies should not break it. For example, it's fairly common in JS to feature-test properties before calling them, like so:

if (obj.prop) {
  obj.prop(...args);
}

"invoke-only" properties, as we called them, would break this idiom. Also, functional idioms like array.map() depend on this equivalence.

@littledan
Copy link
Member

such a trap would give the proxy access to the function it was passed to, which is a new, previously unseen kind of data flow.

Is there a writeup of the threat model that the design of Proxies is trying to defend against? This could be useful in evaluating potential solutions. Proxies allow other sorts of data flows, for example the ability to intercept any property access, which caused an information leakage issue.

I also remember we discussed at that point that the above solution would not be adequate to handle classes with private state, so it would likely have been a half-baked solution anyway.

Was in the context of the previous private symbols proposal? What was the issue?

What I remember as one decisive argument from past discussions is that the invariant that a method call in JS is really just property selection followed by function application is too fundamental to the language semantics, and proxies should not break it.

I see how invocation of methods being based on property access is fundamental to JavaScript, but there are many other properties of objects without Proxies which Proxies were comfortable changing (e.g., side effects from walking up the prototype chain). How were these sorts of distinctions made in the design of Proxies? (I can see some invariants in the spec, but these higher level invariants seem to be out of scope.)

@tvcutsem
Copy link

tvcutsem commented Feb 26, 2018

As far as threat model is concerned: @erights and I primarily considered the object-capability security model and the corresponding four "laws" of information flow (see https://en.wikipedia.org/wiki/Object-capability_model). For property accesses, before symbols were introduced, properties were always benign strings and never capabilities. The fact that a proxy could "steal" a symbol was one reason why symbols were not seen as a solution to enable private state.

The issue with the nonGenericCall trap was that it would only work for a list of pre-identified built-in methods. If a class defines its own methods that handle private state, method invocations on proxies for its instances wouldn't trap using nonGenericCall.

I wish there was a clear-cut answer to what aspects of the language proxies could or could not change. The design of proxies stretched a 3-year period with multiple revisions as we progressively gained more insights into what did and did not work. The most comprehensive write-up of their design is in this tech report (in particular section 5 "Design Principles"). wiki.ecmascript.org also contains (contained? it's no longer responding, using wayback machine links) relevant design notes, see original proposal, revised 'direct proxies' proposal) TC39 did not use GitHub back then. I made a habit of recording meeting notes and outcomes on the proposal page itself.

@erights at one point described a wonderful litmus test for deciding what invariants are worth preserving using proxies. He distinguished between "eternal invariants" and "momentary invariants". That entire discussion thread is actually good context for the invariants that eventually were codified in the ES2015 spec.

@littledan
Copy link
Member

I wrote up a quick README for what it would look like to have opt-in tunneling in Proxies which are used in this way for observation rather than encapsulation: a Proxy.transparent proposal. I'd be interested in your feedback.

@ExE-Boss
Copy link
Contributor

IsArray returning true for proxies around arrays is the correct behaviour, as the proxy will still behave like an Array Exotic Object when the length or a numeric property is written to.

@bathos
Copy link
Contributor

bathos commented Jan 12, 2020

@ExE-Boss if we create new Proxy([], handlers), whether the the result will “behave like an Array Exotic Object” depends entirely on the handlers, which can override any/all of the behaviors of the array exotic object internal methods. The only thing they couldn’t alter is how the configurability of length gets reported, due to invariant enforcements (though they could make accessing it throw). This is due to array construction defining that property as unconfigurable rather than stemming from the AEO internal methods.

For contexts where one actually needs to establish invariant knowledge about a value, Array.isArray / IsArray is pretty much useless due to the passthrough behavior.

@LJ1102
Copy link

LJ1102 commented Dec 15, 2021

Sorry to dig this up, but has there been some conclusion on native methods supporting proxies? I'm concerned with this specifically in regards to DOM methods like Node.prototype.appendNode, should those methods accept proxies or not?

@bakkot
Copy link
Contributor

bakkot commented Dec 15, 2021

@LJ1102 There's been no change to the current state of things, which is that native methods do not pierce proxies (and so e.g. Node.prototype.appendNode throws if invoked on a proxy).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests