Proxy's [[Get]] passes incorrect receiver
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 May 2017 03:51:14 +0000 (03:51 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 May 2017 03:51:14 +0000 (03:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164849
<rdar://problem/31767058>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/proxy-get-set-correct-receiver.js: Added.
(assert):
(test):
(test.let.target.set prop):
(test.let.target.get prop):
(test.get let):
* stress/proxy-set.js:
(let.target.get x):
* stress/reflect-set-proxy-set.js:
(let.target.get x):
* stress/reflect-set-receiver-proxy-set.js:
(let.target.get x):

Source/JavaScriptCore:

* runtime/ProxyObject.cpp:
(JSC::performProxyGet):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@217093 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/proxy-get-set-correct-receiver.js [new file with mode: 0644]
JSTests/stress/proxy-set.js
JSTests/stress/reflect-set-proxy-set.js
JSTests/stress/reflect-set-receiver-proxy-set.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ProxyObject.cpp

index ab23310..feeb7f6 100644 (file)
@@ -1,3 +1,24 @@
+2017-05-18  Saam Barati  <sbarati@apple.com>
+
+        Proxy's [[Get]] passes incorrect receiver
+        https://bugs.webkit.org/show_bug.cgi?id=164849
+        <rdar://problem/31767058>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/proxy-get-set-correct-receiver.js: Added.
+        (assert):
+        (test):
+        (test.let.target.set prop):
+        (test.let.target.get prop):
+        (test.get let):
+        * stress/proxy-set.js:
+        (let.target.get x):
+        * stress/reflect-set-proxy-set.js:
+        (let.target.get x):
+        * stress/reflect-set-receiver-proxy-set.js:
+        (let.target.get x):
+
 2017-05-18  Filip Pizlo  <fpizlo@apple.com>
 
         Constructor calls set this too early
diff --git a/JSTests/stress/proxy-get-set-correct-receiver.js b/JSTests/stress/proxy-get-set-correct-receiver.js
new file mode 100644 (file)
index 0000000..e7fc18c
--- /dev/null
@@ -0,0 +1,117 @@
+"use strict";
+
+function assert(b) {
+    if (!b)
+        throw new Error("Bad")
+}
+
+function test(f, count = 1000) {
+    noInline(f);
+    for (let i = 0; i < count; ++i)
+        f();
+}
+
+test(function() {
+    let called = false;
+    let target = {
+        set prop(x)
+        {
+            assert(x === 20);
+            called = true;
+            assert(this === proxy)
+        }
+    }
+
+    let proxy = new Proxy(target, {})
+    proxy.prop = 20;
+    assert(called);
+});
+
+test(function() {
+    let called = false;
+    let target = {
+        get prop()
+        {
+            called = true;
+            assert(this === proxy)
+        }
+    }
+
+    let proxy = new Proxy(target, {})
+    proxy.prop
+    assert(called);
+});
+
+test(function() {
+    let target = {
+        get prop()
+        {
+            called = true;
+            assert(this === proxy)
+        }
+    }
+    let p1 = new Proxy(target, {});
+
+    let called = false;
+    let proxy = new Proxy(p1, {});
+    proxy.prop
+    assert(called);
+});
+
+test(function() {
+    let t = {};
+    let p1 = new Proxy(t, {
+        get(target, prop, receiver) {
+            called = true;
+            assert(target === t);
+            assert(receiver === proxy);
+            assert(prop === "prop");
+        }
+    });
+
+    let called = false;
+    let proxy = new Proxy(p1, {});
+    proxy.prop
+    assert(called);
+});
+
+test(function() {
+    let t = {};
+    let callCount = 0;
+    let handler = {
+        get(target, prop, receiver) {
+            if (callCount === 200)
+                assert(target === t);
+            ++callCount;
+            assert(receiver === proxy);
+            assert(prop === "prop");
+            return Reflect.get(target, prop, receiver);
+        }
+    };
+    let proxy = new Proxy(t, handler);
+    for (let i = 0; i < 200; ++i)
+        proxy = new Proxy(proxy, handler);
+    proxy.prop
+    assert(callCount === 201);
+}, 10);
+
+test(function() {
+    let t = {};
+    let callCount = 0;
+    let handler = {
+        set(target, prop, value, receiver) {
+            if (callCount === 200)
+                assert(target === t);
+            ++callCount;
+            assert(receiver === proxy);
+            assert(prop === "prop");
+            assert(value === 20);
+            return Reflect.set(target, prop, value, receiver);
+        }
+    };
+    let proxy = new Proxy(t, handler);
+    for (let i = 0; i < 200; ++i)
+        proxy = new Proxy(proxy, handler);
+    proxy.prop = 20;
+    assert(callCount === 201);
+}, 10);
index f55a0b8..2ebb2c1 100644 (file)
@@ -349,6 +349,7 @@ function assert(b) {
 
 {
     let called = false;
+    let throughProxy = false;
     let target = {
         set x(v) {
             assert(this === target);
@@ -356,7 +357,10 @@ function assert(b) {
             called = true;
         },
         get x() {
-            assert(this === target);
+            if (throughProxy)
+                assert(this === proxy);
+            else
+                assert(this === target);
             return this._x;
         }
     };
@@ -374,7 +378,9 @@ function assert(b) {
     for (let i = 0; i < 1000; i++) {
         proxy.x = i;
         assert(called);
+        throughProxy = true;
         assert(proxy.x === i);
+        throughProxy = false;
         assert(target.x === i);
         assert(proxy._x === i);
         assert(target._x === i);
index a655997..4e8abdd 100644 (file)
@@ -364,6 +364,7 @@ function assert(b) {
 
 {
     let called = false;
+    let throughProxy = false;
     let target = {
         set x(v) {
             assert(this === target);
@@ -371,7 +372,10 @@ function assert(b) {
             called = true;
         },
         get x() {
-            assert(this === target);
+            if (throughProxy)
+                assert(this === proxy);
+            else
+                assert(this === target);
             return this._x;
         }
     };
@@ -389,7 +393,9 @@ function assert(b) {
     for (let i = 0; i < 1000; i++) {
         shouldBe(Reflect.set(proxy, 'x', i), true);
         assert(called);
+        throughProxy = true;
         assert(proxy.x === i);
+        throughProxy = false;
         assert(target.x === i);
         assert(proxy._x === i);
         assert(target._x === i);
index 04955c6..054de70 100644 (file)
@@ -368,6 +368,7 @@ function assert(b) {
     }
 
     {
+        let throughProxy = false;
         let called = false;
         let target = {
             set x(v) {
@@ -376,7 +377,10 @@ function assert(b) {
                 called = true;
             },
             get x() {
-                assert(this === target);
+                if (throughProxy)
+                    assert(this === proxy);
+                else
+                    assert(this === target);
                 return this._x;
             }
         };
@@ -395,7 +399,9 @@ function assert(b) {
         for (let i = 0; i < 1000; i++) {
             shouldBe(Reflect.set(proxy, 'x', i, theReceiver), true);
             assert(called);
+            throughProxy = true;
             assert(proxy.x === i);
+            throughProxy = false;
             assert(target.x === i);
             assert(theReceiver.x === undefined);
             assert(proxy._x === i);
index 88f9e88..f456a98 100644 (file)
@@ -1,3 +1,14 @@
+2017-05-18  Saam Barati  <sbarati@apple.com>
+
+        Proxy's [[Get]] passes incorrect receiver
+        https://bugs.webkit.org/show_bug.cgi?id=164849
+        <rdar://problem/31767058>
+
+        Reviewed by Yusuke Suzuki.
+
+        * runtime/ProxyObject.cpp:
+        (JSC::performProxyGet):
+
 2017-05-18  Andy Estes  <aestes@apple.com>
 
         ENABLE(APPLE_PAY_DELEGATE) should be NO on macOS Sierra and earlier
index a9b64ea..1826141 100644 (file)
@@ -128,13 +128,15 @@ static JSValue performProxyGet(ExecState* exec, ProxyObject* proxyObject, JSValu
     JSObject* target = proxyObject->target();
 
     auto performDefaultGet = [&] {
-        return target->get(exec, propertyName);
+        scope.release();
+        PropertySlot slot(receiver, PropertySlot::InternalMethodType::Get);
+        if (target->getPropertySlot(exec, propertyName, slot))
+            return slot.getValue(exec, propertyName);
+        return jsUndefined();
     };
 
-    if (vm.propertyNames->isPrivateName(Identifier::fromUid(&vm, propertyName.uid()))) {
-        scope.release();
+    if (vm.propertyNames->isPrivateName(Identifier::fromUid(&vm, propertyName.uid())))
         return performDefaultGet();
-    }
 
     JSValue handlerValue = proxyObject->handler();
     if (handlerValue.isNull())
@@ -146,10 +148,8 @@ static JSValue performProxyGet(ExecState* exec, ProxyObject* proxyObject, JSValu
     JSValue getHandler = handler->getMethod(exec, callData, callType, vm.propertyNames->get, ASCIILiteral("'get' property of a Proxy's handler object should be callable"));
     RETURN_IF_EXCEPTION(scope, { });
 
-    if (getHandler.isUndefined()) {
-        scope.release();
+    if (getHandler.isUndefined())
         return performDefaultGet();
-    }
 
     MarkedArgumentBuffer arguments;
     arguments.append(target);