Attachment 371906 Details for Bug 177398 – Patch

[patch] Patch

bug-177398-20190612044152.patch (text/plain), 8.54 KB, created by Alexey Shvayka on 2019-06-11 18:41:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Shvayka

Created: 2019-06-11 18:41:53 PDT

Size: 8.54 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 246343)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2019-06-11  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSC should throw if proxy set returns falsish in strict mode context
>+        https://bugs.webkit.org/show_bug.cgi?id=177398
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        1. Add coverage for Proxy `set` trap returning falsy value in strict mode.
>+        2. RegExp methods throw unless [[Set]] succeeds. Return `true` from Proxy `set` traps to fix the tests.
>+
>+        * stress/proxy-set.js: Add 2 test cases.
>+        * stress/regexp-match-proxy.js: Fix test.
>+        * stress/regexp-replace-proxy.js: Fix test.
>+
> 2019-06-11  Alexey Shvayka  <shvaikalesh@gmail.com>
> 
>         Error message for non-callable Proxy `construct` trap is misleading
>Index: JSTests/stress/proxy-set.js
>===================================================================
>--- JSTests/stress/proxy-set.js	(revision 246067)
>+++ JSTests/stress/proxy-set.js	(working copy)
>@@ -80,6 +80,56 @@ function assert(b) {
>     }
> }
> 
>+(function() {
>+    "use strict";
>+    let target = {
>+        x: 30
>+    };
>+
>+    let handler = {
>+        set: function() {
>+            return false;
>+        }
>+    };
>+
>+    let proxy = new Proxy(target, handler);
>+    for (let i = 0; i < 1000; i++) {
>+        let threw = false;
>+        try {
>+            proxy.x = 40;
>+        } catch(e) {
>+            assert(e.toString() === "TypeError: Proxy object's 'set' trap returned falsy value for property 'x'");
>+            threw = true;
>+        }
>+        assert(threw);
>+    }
>+})();
>+
>+(function() {
>+    "use strict";
>+    let target = {
>+        x: 30
>+    };
>+
>+    let handler = {
>+        set: function() {
>+            return false;
>+        }
>+    };
>+
>+    let proxy = new Proxy(target, handler);
>+    for (let i = 0; i < 1000; i++) {
>+        let threw = false;
>+        try {
>+            proxy[42] = 40;
>+        } catch(e) {
>+            assert(e.toString() === "TypeError: Proxy object's 'set' trap returned falsy value for property '42'");
>+            threw = true;
>+        }
>+        assert(threw);
>+    }
>+})();
>+
> {
>     let target = { };
>     Object.defineProperty(target, "x", {
>Index: JSTests/stress/regexp-match-proxy.js
>===================================================================
>--- JSTests/stress/regexp-match-proxy.js	(revision 246067)
>+++ JSTests/stress/regexp-match-proxy.js	(working copy)
>@@ -53,6 +53,7 @@ let getSetProxyNullExec = new Proxy(
>             set.push(k);
>             getSet.push(k);
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>@@ -83,6 +84,7 @@ let getSetProxyMatches_s = new Proxy(
>             set.push(k);
>             getSet.push(k);
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>@@ -117,6 +119,7 @@ let getSetProxyMatches_tx_Greedy = new P
>             if (k.toString() == "lastIndex")
>                 regExpGlobal_tx_Greedy.lastIndex = v;
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>@@ -152,6 +155,7 @@ let getSetProxyMatchesUnicode_digit_nonG
>             if (k.toString() == "lastIndex")
>                 regExpGlobalUnicode_digit_nonGreedy.lastIndex = v;
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>Index: JSTests/stress/regexp-replace-proxy.js
>===================================================================
>--- JSTests/stress/regexp-replace-proxy.js	(revision 246067)
>+++ JSTests/stress/regexp-replace-proxy.js	(working copy)
>@@ -50,6 +50,7 @@ let getSetProxyNullExec = new Proxy(
>         {
>             getSet.push(k);
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>@@ -78,6 +79,7 @@ let getSetProxyMatches_comma = new Proxy
>         {
>             getSet.push(k);
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>@@ -110,6 +112,7 @@ let getSetProxyReplace_phoneNumber = new
>             if (k.toString() == "lastIndex")
>                 regExp_phoneNumber.lastIndex = v;
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>@@ -141,6 +144,7 @@ let getSetProxyReplaceUnicode_digit_nonG
>             if (k.toString() == "lastIndex")
>                 regExpGlobalUnicode_digit_nonGreedy.lastIndex = v;
>             o[k] = v;
>+            return true;
>         }
>     });
> 
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 246302)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2019-06-11  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSC should throw if proxy set returns falsish in strict mode context
>+        https://bugs.webkit.org/show_bug.cgi?id=177398
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Throw TypeError exception if Proxy's `set` trap returns falsy value.
>+        (step 6.c of https://tc39.es/ecma262/#sec-putvalue)
>+
>+        * runtime/ProxyObject.cpp:
>+        (JSC::ProxyObject::performPut):
>+        (JSC::ProxyObject::put):
>+        (JSC::ProxyObject::putByIndexCommon):
>+        * runtime/ProxyObject.h:
>+
> 2019-06-10  Basuke Suzuki  <Basuke.Suzuki@sony.com>
> 
>         [WinCairo] Remove build warning from RemoteInspector.
>Index: Source/JavaScriptCore/runtime/ProxyObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/ProxyObject.cpp	(revision 246067)
>+++ Source/JavaScriptCore/runtime/ProxyObject.cpp	(working copy)
>@@ -409,7 +409,7 @@ bool ProxyObject::getOwnPropertySlotByIn
> }
> 
> template <typename PerformDefaultPutFunction>
>-bool ProxyObject::performPut(ExecState* exec, JSValue putValue, JSValue thisValue, PropertyName propertyName, PerformDefaultPutFunction performDefaultPut)
>+bool ProxyObject::performPut(ExecState* exec, JSValue putValue, JSValue thisValue, PropertyName propertyName, PerformDefaultPutFunction performDefaultPut, bool shouldThrow)
> {
>     NO_TAIL_CALLS();
> 
>@@ -448,8 +448,11 @@ bool ProxyObject::performPut(ExecState* 
>     RETURN_IF_EXCEPTION(scope, false);
>     bool trapResultAsBool = trapResult.toBoolean(exec);
>     RETURN_IF_EXCEPTION(scope, false);
>-    if (!trapResultAsBool)
>+    if (!trapResultAsBool) {
>+        if (shouldThrow)
>+            throwVMTypeError(exec, scope, makeString("Proxy object's 'set' trap returned falsy value for property '", String(propertyName.uid()), "'"));
>         return false;
>+    }
> 
>     PropertyDescriptor descriptor;
>     bool hasProperty = target->getOwnPropertyDescriptor(exec, propertyName, descriptor);
>@@ -478,7 +481,7 @@ bool ProxyObject::put(JSCell* cell, Exec
>         JSObject* target = jsCast<JSObject*>(thisObject->target());
>         return target->methodTable(vm)->put(target, exec, propertyName, value, slot);
>     };
>-    return thisObject->performPut(exec, value, slot.thisValue(), propertyName, performDefaultPut);
>+    return thisObject->performPut(exec, value, slot.thisValue(), propertyName, performDefaultPut, slot.isStrictMode());
> }
> 
> bool ProxyObject::putByIndexCommon(ExecState* exec, JSValue thisValue, unsigned propertyName, JSValue putValue, bool shouldThrow)
>@@ -493,7 +496,7 @@ bool ProxyObject::putByIndexCommon(ExecS
>         PutPropertySlot slot(thisValue, isStrictMode); // We must preserve the "this" target of the putByIndex.
>         return target->methodTable(vm)->put(target, exec, ident.impl(), putValue, slot);
>     };
>-    RELEASE_AND_RETURN(scope, performPut(exec, putValue, thisValue, ident.impl(), performDefaultPut));
>+    RELEASE_AND_RETURN(scope, performPut(exec, putValue, thisValue, ident.impl(), performDefaultPut, shouldThrow));
> }
> 
> bool ProxyObject::putByIndex(JSCell* cell, ExecState* exec, unsigned propertyName, JSValue value, bool shouldThrow)
>Index: Source/JavaScriptCore/runtime/ProxyObject.h
>===================================================================
>--- Source/JavaScriptCore/runtime/ProxyObject.h	(revision 246067)
>+++ Source/JavaScriptCore/runtime/ProxyObject.h	(working copy)
>@@ -100,7 +100,7 @@ private:
>     template <typename DefaultDeleteFunction>
>     bool performDelete(ExecState*, PropertyName, DefaultDeleteFunction);
>     template <typename PerformDefaultPutFunction>
>-    bool performPut(ExecState*, JSValue putValue, JSValue thisValue, PropertyName, PerformDefaultPutFunction);
>+    bool performPut(ExecState*, JSValue putValue, JSValue thisValue, PropertyName, PerformDefaultPutFunction, bool shouldThrow);
>     bool performPreventExtensions(ExecState*);
>     bool performIsExtensible(ExecState*);
>     bool performDefineOwnProperty(ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);

Actions: View | Formatted Diff | Diff

Attachments on bug 177398: 371809 | 371843 | 371847 | 371864 | 371877 | 371906