chakra-core / ChakraCore

ChakraCore is an open source Javascript engine with a C API.
MIT License
9.13k stars 1.2k forks source link

Proxy construct method without trap does not use get trap for prototype #6597

Closed qiudaoyuyesok closed 3 years ago

qiudaoyuyesok commented 3 years ago
Version
Chakra-1.11.24
Execution step
./ChakraCore/out/Release/ch testcase.js
Testcase
target = Function();
handler = {
    get: function() {
        print('Run here');
    }
};
var p= new Proxy(target, handler);
new p;
Output
Expected behavior
Run here
Description

When executing the above test case, the code creates a proxy for an anonymous function named "p" in line 4. When executing the last line of code, other engines (spidermonkey, v8, JavascriptCore, etc.) will call the get function defined in line 3, but Chakra did not do so. It seems that the implementation of chakra is incorrect.

ljharb commented 3 years ago

What arguments is the get handler invoked with in the other engines?

rhuanjl commented 3 years ago

Thanks for the tip on looking at the arguments to get @ljharb

I had a quick look starting by printing the trap arguments, what it's meant to be geting is the prototype for the constructor.

There's an error in JavascriptProxy::FunctionCallTrap in lib/Runtime/Library/JavascriptProxy.cpp

When there's no construct trap on the proxy it calls the target with JavascriptFunction::CallFunction when I think it needs to use JavascriptFunction::CallAsConstructor, as a result of using the wrong method it never attempts to get the prototype - somehow it's gifted it anyway but it evades the get trap in so doing. Fix will involve adjusting the logic in that function to select the right method depending on context - there's a few different code paths that go through there.

Working through the logic may be slightly tricky BUT actual fix should be just getting conditions right and changing the function call under the right conditions - all contained in that one method - so perhaps a good first issue if anyone will take it.

POC of the full issue:

let got = false;
function bar () {print ('in the constructor');}
bar.prototype.a = "Should not have this";

const target = bar;
print(Object.getOwnPropertyNames(target.prototype));
let  handler = {
      get: function(...args) {
          if (!got)
          {
            got = true;
            print (JSON.stringify(args));

          }
          return {};
      }
  };
  var p= new Proxy(target, handler);

print((new p).a);

eshost output:

eshost out/testcase.js -s
#### Chakra, chDev
constructor,a
in the constructor
Should not have this

#### JavaScriptCore, SpiderMonkey, V8
constructor,a
[null,"prototype",null]
in the constructor
undefined
rhuanjl commented 3 years ago

Actual fix was not what I'd written above; CallAsConstructor would not have worked, had to update the separate object creation step instead.