RESOLVED FIXED 164849
Proxy's [[Get]] passes incorrect receiver
https://bugs.webkit.org/show_bug.cgi?id=164849
Summary Proxy's [[Get]] passes incorrect receiver
Alexey Shvayka
Reported 2016-11-16 17:55:30 PST
Please consider the following code: ``` var target = { get prop() { console.log(this == proxy) // => `false`, should be `true` } } var proxy = new Proxy(target, {}) proxy.prop ``` `proxy.prop` calls `[[Get]]` on `proxy` with `"prop"` as key and `proxy` as receiver. Proxy's `[[Get]]` method checks for `get` trap, it is missing, so it should call `[[Get]]` on `target` with **the same** parameters. However, JSC does not pass receiver, thus `prop` getter is called with context of `target`, not `proxy`. Both V8 and SpiderMonkey implement this correctly. tc39/test-262 PR: https://github.com/tc39/test262/pull/792 chai/chaijs issue: https://github.com/chaijs/chai/issues/855
Attachments
patch (5.70 KB, patch)
2017-05-18 17:35 PDT, Saam Barati
no flags
patch (5.61 KB, patch)
2017-05-18 17:36 PDT, Saam Barati
ysuzuki: review+
buildbot: commit-queue-
patch for landing (9.05 KB, patch)
2017-05-18 18:23 PDT, Saam Barati
no flags
Radar WebKit Bug Importer
Comment 1 2017-04-21 15:07:13 PDT
Saam Barati
Comment 2 2017-05-18 17:35:04 PDT
Saam Barati
Comment 3 2017-05-18 17:36:16 PDT
Comment on attachment 310579 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=310579&action=review > JSTests/stress/proxy-get-set-correct-receiver.js:21 > + assert(this === proxy) // => `false`, should be `true` oops, let me remove these comments.
Saam Barati
Comment 4 2017-05-18 17:36:49 PDT
Yusuke Suzuki
Comment 5 2017-05-18 17:41:07 PDT
Comment on attachment 310580 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=310580&action=review r=me > JSTests/stress/proxy-get-set-correct-receiver.js:36 > + assert(this === proxy) OK, receiver is proxy. > JSTests/stress/proxy-get-set-correct-receiver.js:50 > + assert(this === proxy) OK, receiver is neither p1 nor target. > Source/JavaScriptCore/runtime/ProxyObject.cpp:135 > + return jsUndefined(); OK, previously, we ignored receiver.
Build Bot
Comment 6 2017-05-18 18:15:47 PDT
Comment on attachment 310580 [details] patch Attachment 310580 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/3772712 New failing tests: stress/proxy-set.js.ftl-no-cjit-small-pool stress/reflect-set-proxy-set.js.ftl-no-cjit-no-put-stack-validate stress/reflect-set-receiver-proxy-set.js.default stress/reflect-set-receiver-proxy-set.js.no-llint stress/reflect-set-receiver-proxy-set.js.ftl-no-cjit-validate-sampling-profiler stress/reflect-set-proxy-set.js.ftl-no-cjit-b3o1 stress/reflect-set-receiver-proxy-set.js.ftl-eager stress/reflect-set-proxy-set.js.ftl-no-cjit-small-pool stress/reflect-set-receiver-proxy-set.js.ftl-eager-no-cjit-b3o1 stress/reflect-set-proxy-set.js.ftl-no-cjit-validate-sampling-profiler stress/proxy-set.js.ftl-no-cjit-validate-sampling-profiler stress/proxy-set.js.ftl-eager-no-cjit-b3o1 stress/reflect-set-proxy-set.js.ftl-eager-no-cjit stress/reflect-set-receiver-proxy-set.js.ftl-eager-no-cjit stress/proxy-set.js.ftl-eager stress/reflect-set-proxy-set.js.ftl-no-cjit-no-inline-validate stress/proxy-set.js.dfg-eager stress/reflect-set-proxy-set.js.dfg-eager stress/proxy-set.js.ftl-no-cjit-no-inline-validate stress/proxy-set.js.ftl-no-cjit-b3o1 stress/proxy-set.js.no-llint stress/reflect-set-proxy-set.js.ftl-eager stress/reflect-set-proxy-set.js.dfg-eager-no-cjit-validate stress/proxy-set.js.default stress/proxy-set.js.no-cjit-validate-phases stress/reflect-set-proxy-set.js.no-cjit-validate-phases stress/reflect-set-receiver-proxy-set.js.ftl-no-cjit-no-put-stack-validate stress/reflect-set-receiver-proxy-set.js.dfg-eager-no-cjit-validate stress/reflect-set-receiver-proxy-set.js.dfg-eager stress/reflect-set-receiver-proxy-set.js.ftl-no-cjit-no-inline-validate stress/proxy-set.js.ftl-eager-no-cjit stress/reflect-set-proxy-set.js.default stress/proxy-set.js.dfg-eager-no-cjit-validate stress/reflect-set-proxy-set.js.no-llint stress/proxy-set.js.ftl-no-cjit-no-put-stack-validate stress/reflect-set-receiver-proxy-set.js.no-cjit-validate-phases stress/reflect-set-proxy-set.js.no-cjit-collect-continuously stress/proxy-set.js.no-ftl stress/reflect-set-proxy-set.js.dfg-maximal-flush-validate-no-cjit stress/reflect-set-proxy-set.js.no-ftl stress/reflect-set-receiver-proxy-set.js.no-ftl stress/reflect-set-receiver-proxy-set.js.ftl-no-cjit-b3o1 stress/proxy-set.js.dfg-maximal-flush-validate-no-cjit stress/reflect-set-receiver-proxy-set.js.ftl-no-cjit-small-pool stress/reflect-set-receiver-proxy-set.js.dfg-maximal-flush-validate-no-cjit stress/reflect-set-proxy-set.js.ftl-eager-no-cjit-b3o1 stress/proxy-set.js.no-cjit-collect-continuously stress/reflect-set-receiver-proxy-set.js.no-cjit-collect-continuously
Saam Barati
Comment 7 2017-05-18 18:23:45 PDT
Created attachment 310587 [details] patch for landing
WebKit Commit Bot
Comment 8 2017-05-18 20:51:17 PDT
Comment on attachment 310587 [details] patch for landing Clearing flags on attachment: 310587 Committed r217093: <http://trac.webkit.org/changeset/217093>
WebKit Commit Bot
Comment 9 2017-05-18 20:51:19 PDT
All reviewed patches have been landed. Closing bug.
GSkachkov
Comment 10 2017-05-30 12:16:26 PDT
*** Bug 169040 has been marked as a duplicate of this bug. ***
GSkachkov
Comment 11 2017-05-30 12:18:10 PDT
*** Bug 171915 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.