Attachment 372107 Details for Bug 198630 – Revert EXCEPTION_ASSERT change and add isExtensible test.

[patch] Revert EXCEPTION_ASSERT change and add isExtensible test.

bug-198630-20190614083222.patch (text/plain), 6.69 KB, created by Alexey Shvayka on 2019-06-13 22:32:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Shvayka

Created: 2019-06-13 22:32:23 PDT

Size: 6.69 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 246427)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-06-13  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        Three checks are missing in Proxy internal methods
>+        https://bugs.webkit.org/show_bug.cgi?id=198630
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/proxy-delete.js: Assert isExtensible is called in the correct order.
>+        * test262/expectations.yaml:
>+
> 2019-06-13  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Yarr bytecode compilation failure should be gracefully handled
>Index: JSTests/stress/proxy-delete.js
>===================================================================
>--- JSTests/stress/proxy-delete.js	(revision 246067)
>+++ JSTests/stress/proxy-delete.js	(working copy)
>@@ -93,6 +93,22 @@ function assert(b) {
> }
> 
> {
>+    let target = new Proxy({}, {
>+        isExtensible: function() {
>+            throw new Error("should not be called if [[GetOwnProperty]] returns undefined");
>+        }
>+    });
>+    let proxy = new Proxy(target, {
>+        deleteProperty: function() {
>+            return true;
>+        }
>+    });
>+    for (let i = 0; i < 500; i++) {
>+        assert(delete proxy.nonExistentProperty);
>+    }
>+}
>+
>+{
>     let target = {};
>     let error = null;
>     let handler = {
>Index: JSTests/test262/expectations.yaml
>===================================================================
>--- JSTests/test262/expectations.yaml	(revision 246067)
>+++ JSTests/test262/expectations.yaml	(working copy)
>@@ -1143,15 +1143,6 @@ test/built-ins/Proxy/construct/trap-is-u
> test/built-ins/Proxy/create-handler-is-revoked-proxy.js:
>   default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>   strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-test/built-ins/Proxy/defineProperty/targetdesc-not-configurable-writable-desc-not-writable.js:
>-  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-test/built-ins/Proxy/deleteProperty/targetdesc-is-configurable-target-is-not-extensible.js:
>-  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-test/built-ins/Proxy/getOwnPropertyDescriptor/resultdesc-is-not-configurable-not-writable-targetdesc-is-writable.js:
>-  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
> test/built-ins/RegExp/named-groups/groups-object-subclass-sans.js:
>   default: 'Test262Error: Expected SameValue(ÃÂ«bÃÂ», ÃÂ«$<a>ÃÂ») to be true'
>   strict mode: 'Test262Error: Expected SameValue(ÃÂ«bÃÂ», ÃÂ«$<a>ÃÂ») to be true'
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 246417)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2019-06-13  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        Three checks are missing in Proxy internal methods
>+        https://bugs.webkit.org/show_bug.cgi?id=198630
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add three missing checks in Proxy internal methods.
>+        These checks are needed to maintain the invariants of the essential internal methods.
>+        (https://github.com/tc39/ecma262/pull/666)
>+
>+        1. [[GetOwnProperty]] shouldn't return non-configurable and non-writable descriptor when the target's property is writable.
>+        2. [[Delete]] should return `false` when the target has property and is not extensible.
>+        3. [[DefineOwnProperty]] should return `true` for a non-writable input descriptor when the target's property is non-configurable and writable.
>+
>+        * runtime/ProxyObject.cpp:
>+        (JSC::ProxyObject::performInternalMethodGetOwnProperty): Add writability check.
>+        (JSC::ProxyObject::performDelete): Add extensibility check.
>+        (JSC::ProxyObject::performDefineOwnProperty): Add writability check.
>+
> 2019-06-13  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Yarr bytecode compilation failure should be gracefully handled
>Index: Source/JavaScriptCore/runtime/ProxyObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/ProxyObject.cpp	(revision 246067)
>+++ Source/JavaScriptCore/runtime/ProxyObject.cpp	(working copy)
>@@ -287,6 +287,10 @@ bool ProxyObject::performInternalMethodG
>             throwVMTypeError(exec, scope, "Result from 'getOwnPropertyDescriptor' can't be non-configurable when the 'target' doesn't have it as an own property or if it is a configurable own property on 'target'"_s);
>             return false;
>         }
>+        if (trapResultAsDescriptor.writablePresent() && !trapResultAsDescriptor.writable() && targetPropertyDescriptor.writable()) {
>+            throwVMTypeError(exec, scope, "Result from 'getOwnPropertyDescriptor' can't be non-configurable and non-writable when the target's property is writable"_s);
>+            return false;
>+        }
>     }
> 
>     if (trapResultAsDescriptor.isAccessorDescriptor()) {
>@@ -659,6 +663,12 @@ bool ProxyObject::performDelete(ExecStat
>             throwVMTypeError(exec, scope, "Proxy handler's 'deleteProperty' method should return false when the target's property is not configurable"_s);
>             return false;
>         }
>+        bool targetIsExtensible = target->isExtensible(exec);
>+        RETURN_IF_EXCEPTION(scope, false);
>+        if (!targetIsExtensible) {
>+            throwVMTypeError(exec, scope, "Proxy handler's 'deleteProperty' method should return false when the target has property and is not extensible"_s);
>+            return false;
>+        }
>     }
> 
>     RETURN_IF_EXCEPTION(scope, false);
>@@ -883,6 +893,12 @@ bool ProxyObject::performDefineOwnProper
>         throwVMTypeError(exec, scope, "Proxy's 'defineProperty' trap did not define a non-configurable property on its target even though the input descriptor to the trap said it must do so"_s);
>         return false;
>     }
>+    if (targetDescriptor.isDataDescriptor() && !targetDescriptor.configurable() && targetDescriptor.writable()) {
>+        if (descriptor.writablePresent() && !descriptor.writable()) {
>+            throwTypeError(exec, scope, "Proxy's 'defineProperty' trap returned true for a non-writable input descriptor when the target's property is non-configurable and writable"_s);
>+            return false;
>+        }
>+    }
>     
>     return true;
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 198630: 371538 | 371546 | 371547 | 372107 | 372155 | 372174 | 374819