claudepache / es-legacy-function-reflection

JavaScript: Legacy reflection features on functions needed for web compatibility
Creative Commons Zero v1.0 Universal
3 stars 2 forks source link

What happens when setting to .caller or .arguments? #9

Closed claudepache closed 4 years ago

claudepache commented 4 years ago

We should check what happens in the following cases:

function noncensored() { }
function censored() { "use strict"; }

noncensored.caller = 42;
censored.caller = 42;

(function () { 
    "use strict";
    noncensored.caller = 42;
    censored.caller = 42;
})()
hax commented 4 years ago

All throw TypeError?

ljharb commented 4 years ago

Even if there was a property descriptor that had a throwing setter, I'd expect it should be completely possible to defineProperty a caller property onto any function.

claudepache commented 4 years ago

The currently specced behaviour is well understood:

function noncensored() { }
function censored() { "use strict"; }

noncensored.caller = 42; // fails silently
censored.caller = 42; // fails silently

(function () { 
    "use strict";
    noncensored.caller = 42; // throws a TypeError
    censored.caller = 42;// throws a TypeError
})()

(and, of course, everything redefinable with defineProperty).

I meant: We should check what happens in current implementations and compare.

hax commented 4 years ago

All engines throw for censored.caller = 42, it seems it's the behavior from ES5 era when we add strict mode?

hax commented 4 years ago

Current engines (V8, SpiderMonkey, JSC and Chakra) behavior:

function noncensored() { }
function censored() { "use strict"; }

noncensored.caller = 42; // fails silently
censored.caller = 42; // throws a TypeError

(function () { 
    "use strict";
    noncensored.caller = 42; // throws a TypeError except SpiderMonkey (fails silently)
    censored.caller = 42;// throws a TypeError
})()
claudepache commented 4 years ago

So, we have:

⛔️ = assignment fails silently πŸ’₯ = a TypeError is thrown

Β Β Β  (1) (2) (3)
uncensored.caller = 42 ⛔️ ⛔️ ⛔️
censored.caller = 42 πŸ’₯ πŸ’₯ ⛔️
"use strict"; uncensored.caller = 42 πŸ’₯ ⛔️ πŸ’₯
"use strict"; censored.caller = 42 πŸ’₯ πŸ’₯ πŸ’₯

(1) own properties, poisoned setter on censored function (V8, JSC, Chakra) (2) shared accessor on Function.prototype, conditionally poisoned setter (SpiderMonkey) (3) shared accessor on Function.prototype, no setter (proposed spec)

Technically, with a shared accessor, (since always throwing is out of question,) we have to choose between always throwing on censored functions (2), and always throwing in strict mode (3).

I think that it is more important to always throw for failed assignment in strict mode.

ljharb commented 4 years ago

Why is option 1 not better? 3 out of 4 throws is better than 2.

claudepache commented 4 years ago

@ljharb Option 1 implies that we have to load each censored function (including each built-in and each user-created strict function) with a poisoned setter.

The goal would be that a particular failing property assignment in sloppy mode will throw, whereas:

Really, I don’t think that it has much value. I think it is more worthy to reduce the place taken by the misfeature to just one getter per realm.

hax commented 4 years ago

Agree with @claudepache . If caller is accessor, we'd better keep it simple and do not introduce magic again, so the only possibilities are: A. a setter always throw B. no setter (fail silently in non-strict code)

Consider option A have bigger potential risk (fail silent -> throw VS throw -> fail silent), it seems option B is better.