[JSC] Object.keys() must discard property names with no PropertyDescriptor
authorcaitp@igalia.com <caitp@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Apr 2017 14:56:53 +0000 (14:56 +0000)
committercaitp@igalia.com <caitp@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Apr 2017 14:56:53 +0000 (14:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171291

Reviewed by Yusuke Suzuki.

JSTests:

* es6/Proxy_ownKeys_duplicates.js:
* stress/proxy-own-keys.js:
(let.handler.getOwnPropertyDescriptor):
(let.handler.ownKeys):

Source/JavaScriptCore:

Proxy objects can produce an arbitrary list of property names from the
"ownKeys" trap, however the Object.keys() algorithm is required to
discard names which do not have a PropertyDescriptor. This also
applies to other uses of the EnumerableOwnProperties() algorithm
(https://tc39.github.io/ecma262/#sec-enumerableownproperties)

Related to https://bugs.chromium.org/p/v8/issues/detail?id=6290

* runtime/ObjectConstructor.cpp:
(JSC::ownPropertyKeys):

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

JSTests/ChangeLog
JSTests/es6/Proxy_ownKeys_duplicates.js
JSTests/stress/proxy-own-keys.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ObjectConstructor.cpp

index ff52a14..04145bc 100644 (file)
@@ -1,3 +1,15 @@
+2017-04-26  Caitlin Potter  <caitp@igalia.com>
+
+        [JSC] Object.keys() must discard property names with no PropertyDescriptor
+        https://bugs.webkit.org/show_bug.cgi?id=171291
+
+        Reviewed by Yusuke Suzuki.
+
+        * es6/Proxy_ownKeys_duplicates.js:
+        * stress/proxy-own-keys.js:
+        (let.handler.getOwnPropertyDescriptor):
+        (let.handler.ownKeys):
+
 2017-04-25  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r215476.
index 45dad96..d4f96ab 100644 (file)
@@ -2,6 +2,13 @@ function test() {
 
 var symbol = Symbol("test");
 var proxy = new Proxy({}, {
+    getOwnPropertyDescriptor(t, n) {
+        // Required to prevent Object.keys() from discarding results
+        return {
+            enumerable: true,
+            configurable: true
+        };
+    },
     ownKeys: function (t) {
         return ["A", "A", "0", "0", symbol, symbol];
     }
index 95da9e6..189b20b 100644 (file)
@@ -308,6 +308,39 @@ function shallowEq(a, b) {
     for (let i = 0; i < 500; i++) {
         let result = Object.keys(proxy);
         assert(result !== arr);
+        assert(shallowEq(result, []));
+        assert(called);
+        called = false;
+    }
+}
+
+{
+    let target = {
+        x: 40
+    };
+    let called = false;
+    let arr = ["a", "b", "c"];
+    let handler = {
+        getOwnPropertyDescriptor: function(theTarget, propertyName) {
+            if (arr.indexOf(propertyName) >= 0) {
+                return {
+                    enumerable: true,
+                    configurable: true
+                };
+            }
+            return Reflect.getOwnPropertyDescriptor(theTarget, propertyName);
+        },
+
+        ownKeys: function(theTarget) {
+            called = true;
+            return arr;
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    for (let i = 0; i < 500; i++) {
+        let result = Object.keys(proxy);
+        assert(result !== arr);
         assert(shallowEq(result, arr));
         assert(called);
         called = false;
@@ -370,6 +403,15 @@ function shallowEq(a, b) {
     let s2 = Symbol();
     let arr = ["a", "b", s1, "c", s2];
     let handler = {
+        getOwnPropertyDescriptor(theTarget, propertyName) {
+            if (arr.indexOf(propertyName) >= 0) {
+                return {
+                    enumerable: true,
+                    configurable: true
+                }
+            }
+            return Reflect.getOwnPropertyDescriptor(theTarget, propertyName);
+        },
         ownKeys: function(theTarget) {
             called = true;
             return arr;
index 2580f96..a180e05 100644 (file)
@@ -1,3 +1,21 @@
+2017-04-26  Caitlin Potter  <caitp@igalia.com>
+
+        [JSC] Object.keys() must discard property names with no PropertyDescriptor
+        https://bugs.webkit.org/show_bug.cgi?id=171291
+
+        Reviewed by Yusuke Suzuki.
+
+        Proxy objects can produce an arbitrary list of property names from the
+        "ownKeys" trap, however the Object.keys() algorithm is required to
+        discard names which do not have a PropertyDescriptor. This also
+        applies to other uses of the EnumerableOwnProperties() algorithm
+        (https://tc39.github.io/ecma262/#sec-enumerableownproperties)
+
+        Related to https://bugs.chromium.org/p/v8/issues/detail?id=6290
+
+        * runtime/ObjectConstructor.cpp:
+        (JSC::ownPropertyKeys):
+
 2017-04-25  Andy VanWagoner  <thetalecrafter@gmail.com>
 
         Unhandled enumeration values in IntlDateTimeFormat.cpp
index cae5cae..befda4e 100644 (file)
@@ -730,14 +730,28 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
     JSArray* keys = constructEmptyArray(exec, 0);
     RETURN_IF_EXCEPTION(scope, nullptr);
 
+    // https://tc39.github.io/ecma262/#sec-enumerableownproperties
+    // If {object} is a Proxy, an explicit and observable [[GetOwnProperty]] op is required to filter out non-enumerable properties.
+    // In other cases, filtering has already been performed.
+    const bool mustFilterProperty = dontEnumPropertiesMode == DontEnumPropertiesMode::Exclude && object->type() == ProxyObjectType;
+    auto filterPropertyIfNeeded = [exec, object, mustFilterProperty](const Identifier& identifier) {
+        if (!mustFilterProperty)
+            return true;
+        PropertyDescriptor descriptor;
+        PropertyName name(identifier);
+        return object->getOwnPropertyDescriptor(exec, name, descriptor) && descriptor.enumerable();
+    };
+
     switch (propertyNameMode) {
     case PropertyNameMode::Strings: {
         size_t numProperties = properties.size();
         for (size_t i = 0; i < numProperties; i++) {
             const auto& identifier = properties[i];
             ASSERT(!identifier.isSymbol());
-            keys->push(exec, jsOwnedString(exec, identifier.string()));
-            RETURN_IF_EXCEPTION(scope, nullptr);
+            if (filterPropertyIfNeeded(identifier)) {
+                keys->push(exec, jsOwnedString(exec, identifier.string()));
+                RETURN_IF_EXCEPTION(scope, nullptr);
+            }
         }
         break;
     }
@@ -747,7 +761,7 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
         for (size_t i = 0; i < numProperties; i++) {
             const auto& identifier = properties[i];
             ASSERT(identifier.isSymbol());
-            if (!vm.propertyNames->isPrivateName(identifier)) {
+            if (!vm.propertyNames->isPrivateName(identifier) && filterPropertyIfNeeded(identifier)) {
                 keys->push(exec, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
                 RETURN_IF_EXCEPTION(scope, nullptr);
             }
@@ -763,7 +777,7 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
             if (identifier.isSymbol()) {
                 if (!vm.propertyNames->isPrivateName(identifier))
                     propertySymbols.append(identifier);
-            } else {
+            } else if (filterPropertyIfNeeded(identifier)) {
                 keys->push(exec, jsOwnedString(exec, identifier.string()));
                 RETURN_IF_EXCEPTION(scope, nullptr);
             }
@@ -771,8 +785,10 @@ JSArray* ownPropertyKeys(ExecState* exec, JSObject* object, PropertyNameMode pro
 
         // To ensure the order defined in the spec (9.1.12), we append symbols at the last elements of keys.
         for (const auto& identifier : propertySymbols) {
-            keys->push(exec, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
-            RETURN_IF_EXCEPTION(scope, nullptr);
+            if (filterPropertyIfNeeded(identifier)) {
+                keys->push(exec, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
+                RETURN_IF_EXCEPTION(scope, nullptr);
+            }
         }
 
         break;