[JSC] Don't reference the properties of @Reflect directly
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Mar 2016 04:08:06 +0000 (04:08 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Mar 2016 04:08:06 +0000 (04:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155436

Reviewed by Geoffrey Garen.

Reflect.ownKeys and Reflect.getOwnPropertyDescriptor can be altered with the user-crafted values.
Instead of referencing them directly, let's reference them through private names.

* builtins/ObjectConstructor.js:
(assign):
* runtime/CommonIdentifiers.h:
* runtime/ObjectConstructor.cpp:
(JSC::ObjectConstructor::finishCreation): Deleted.
* runtime/ReflectObject.cpp:
(JSC::ReflectObject::finishCreation):
* tests/stress/object-assign-correctness.js:
(runTests.):
(runTests.get let):
(Reflect.ownKeys):
(Reflect.getOwnPropertyDescriptor):
(test.let.handler.switch.case.string_appeared_here.return.get enumerable): Deleted.
(test.let.handler.getOwnPropertyDescriptor): Deleted.
(test.let.handler.ownKeys): Deleted.
(test.let.handler.get getProps): Deleted.
(test.let.handler): Deleted.
(test): Deleted.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ObjectConstructor.js
Source/JavaScriptCore/runtime/CommonIdentifiers.h
Source/JavaScriptCore/runtime/ObjectConstructor.cpp
Source/JavaScriptCore/runtime/ReflectObject.cpp
Source/JavaScriptCore/tests/stress/object-assign-correctness.js

index a3d1f37..1b48cae 100644 (file)
@@ -1,3 +1,32 @@
+2016-03-14  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Don't reference the properties of @Reflect directly
+        https://bugs.webkit.org/show_bug.cgi?id=155436
+
+        Reviewed by Geoffrey Garen.
+
+        Reflect.ownKeys and Reflect.getOwnPropertyDescriptor can be altered with the user-crafted values.
+        Instead of referencing them directly, let's reference them through private names.
+
+        * builtins/ObjectConstructor.js:
+        (assign):
+        * runtime/CommonIdentifiers.h:
+        * runtime/ObjectConstructor.cpp:
+        (JSC::ObjectConstructor::finishCreation): Deleted.
+        * runtime/ReflectObject.cpp:
+        (JSC::ReflectObject::finishCreation):
+        * tests/stress/object-assign-correctness.js:
+        (runTests.):
+        (runTests.get let):
+        (Reflect.ownKeys):
+        (Reflect.getOwnPropertyDescriptor):
+        (test.let.handler.switch.case.string_appeared_here.return.get enumerable): Deleted.
+        (test.let.handler.getOwnPropertyDescriptor): Deleted.
+        (test.let.handler.ownKeys): Deleted.
+        (test.let.handler.get getProps): Deleted.
+        (test.let.handler): Deleted.
+        (test): Deleted.
+
 2016-03-14  Daniel Bates  <dabates@apple.com>
 
         Web Inspector: Display Content Security Policy hash in details sidebar for script and style elements
index a17e62c..408979b 100644 (file)
@@ -35,10 +35,10 @@ function assign(target/*[*/, /*...*/sources/*] */)
         let nextSource = arguments[s];
         if (nextSource != null) {
             let from = @Object(nextSource);
-            let keys = @Reflect.ownKeys(from);
+            let keys = @Reflect.@ownKeys(from);
             for (let i = 0, keysLength = keys.length; i < keysLength; ++i) {
                 let nextKey = keys[i];
-                let descriptor = @Reflect.getOwnPropertyDescriptor(from, nextKey);
+                let descriptor = @Reflect.@getOwnPropertyDescriptor(from, nextKey);
                 if (descriptor !== @undefined && descriptor.enumerable)
                     objTarget[nextKey] = from[nextKey];
             }
index c3825ff..85f4545 100644 (file)
     macro(isFinite) \
     macro(isNaN) \
     macro(getPrototypeOf) \
+    macro(getOwnPropertyDescriptor) \
     macro(getOwnPropertyNames) \
+    macro(ownKeys) \
     macro(RangeError) \
     macro(TypeError) \
     macro(typedArrayLength) \
index d2e9992..f032b9b 100644 (file)
@@ -70,6 +70,7 @@ const ClassInfo ObjectConstructor::s_info = { "Function", &InternalFunction::s_i
   getOwnPropertyDescriptor  objectConstructorGetOwnPropertyDescriptor   DontEnum|Function 2
   getOwnPropertyDescriptors objectConstructorGetOwnPropertyDescriptors  DontEnum|Function 1
   getOwnPropertyNames       objectConstructorGetOwnPropertyNames        DontEnum|Function 1
+  getOwnPropertySymbols     objectConstructorGetOwnPropertySymbols      DontEnum|Function 1
   keys                      objectConstructorKeys                       DontEnum|Function 1
   defineProperty            objectConstructorDefineProperty             DontEnum|Function 3
   defineProperties          objectConstructorDefineProperties           DontEnum|Function 2
@@ -98,7 +99,6 @@ void ObjectConstructor::finishCreation(VM& vm, JSGlobalObject* globalObject, Obj
     // no. of arguments for constructor
     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(1), ReadOnly | DontEnum | DontDelete);
 
-    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("getOwnPropertySymbols", objectConstructorGetOwnPropertySymbols, DontEnum, 1);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->getPrototypeOfPrivateName, objectConstructorGetPrototypeOf, DontEnum, 1);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->getOwnPropertyNamesPrivateName, objectConstructorGetOwnPropertyNames, DontEnum, 1);
 }
index c6d57b3..099ac85 100644 (file)
@@ -80,10 +80,13 @@ ReflectObject::ReflectObject(VM& vm, Structure* structure)
 {
 }
 
-void ReflectObject::finishCreation(VM& vm, JSGlobalObject*)
+void ReflectObject::finishCreation(VM& vm, JSGlobalObject* globalObject)
 {
     Base::finishCreation(vm);
     ASSERT(inherits(info()));
+
+    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->ownKeysPrivateName, reflectObjectOwnKeys, DontEnum | DontDelete | ReadOnly, 1);
+    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->getOwnPropertyDescriptorPrivateName, reflectObjectGetOwnPropertyDescriptor, DontEnum | DontDelete | ReadOnly, 2);
 }
 
 bool ReflectObject::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot &slot)
index be067bb..530e3ab 100644 (file)
@@ -7,162 +7,174 @@ function test(f) {
         f();
 }
 
-test(function() {
-    let ownKeysCalled = false;
-    let getOwnPropertyDescriptorProps = [];
-    let getProps = [];
-    let enumerableCalled = false;
-    let handler = {
-        getOwnPropertyDescriptor: function(target, key) { 
-            getOwnPropertyDescriptorProps.push(key);
-            switch(key) {
-            case "foo":
-                return {
-                    enumerable: true,
-                    configurable: true,
-                    value: 45
-                };
-            case "bar":
-                return  {
-                    enumerable: true,
-                    get enumerable() {
-                        enumerableCalled = true;
-                        return true;
-                    },
-                    configurable: true,
-                    value: 50
+var originalReflect = Reflect;
+var ownKeys = Reflect.ownKeys;
+var getOwnPropertyDescriptor = Reflect.getOwnPropertyDescriptor;
+
+function runTests() {
+    test(function() {
+        let ownKeysCalled = false;
+        let getOwnPropertyDescriptorProps = [];
+        let getProps = [];
+        let enumerableCalled = false;
+        let handler = {
+            getOwnPropertyDescriptor: function(target, key) {
+                getOwnPropertyDescriptorProps.push(key);
+                switch(key) {
+                case "foo":
+                    return {
+                        enumerable: true,
+                        configurable: true,
+                        value: 45
+                    };
+                case "bar":
+                    return  {
+                        enumerable: true,
+                        get enumerable() {
+                            enumerableCalled = true;
+                            return true;
+                        },
+                        configurable: true,
+                        value: 50
+                    }
+                case "baz":
+                    return  {
+                        enumerable: false,
+                        configurable: true,
+                        value: 50
+                    }
+                default:
+                    assert(false, "should not be reached.");
+                    break;
                 }
-            case "baz":
-                return  {
-                    enumerable: false,
-                    configurable: true,
-                    value: 50
+            },
+            ownKeys: function(target) {
+                ownKeysCalled = true;
+                return ["foo", "bar", "baz"];
+            },
+            get: function(target, key) {
+                getProps.push(key);
+                switch(key) {
+                case "foo":
+                    return 20;
+                case "bar":
+                    return "bar";
+                default:
+                    assert(false, "should not be reached.");
+                    break;
                 }
-            default:
-                assert(false, "should not be reached.");
-                break;
             }
-        },
-        ownKeys: function(target) {
-            ownKeysCalled = true;
-            return ["foo", "bar", "baz"];
-        },
-        get: function(target, key) {
-            getProps.push(key);
-            switch(key) {
-            case "foo":
-                return 20;
-            case "bar":
-                return "bar";
-            default:
-                assert(false, "should not be reached.");
-                break;
-            }
-        }
-    };
-
-    let proxy = new Proxy({}, handler);
-    let foo = {};
-    Object.assign(foo, proxy);
-
-    assert(enumerableCalled);
-
-    assert(Reflect.ownKeys(foo).length === 2);
-    assert(Reflect.ownKeys(foo)[0] === "foo");
-    assert(Reflect.ownKeys(foo)[1] === "bar");
-    assert(foo.foo === 20);
-    assert(foo.bar === "bar");
-
-    assert(ownKeysCalled);
-    assert(getOwnPropertyDescriptorProps.length === 3);
-    assert(getOwnPropertyDescriptorProps[0] === "foo");
-    assert(getOwnPropertyDescriptorProps[1] === "bar");
-    assert(getOwnPropertyDescriptorProps[2] === "baz");
-
-    assert(getProps.length === 2);
-    assert(getProps[0] === "foo");
-    assert(getProps[1] === "bar");
-});
-
-
-let oldReflect = Reflect;
-Reflect = null;
-assert(Reflect === null); // Make sure Object.assign's use of Reflect is safe.
-
-test(function() {
-    let ownKeysCalled = false;
-    let getOwnPropertyDescriptorProps = [];
-    let getProps = [];
-    let enumerableCalled = false;
-    let handler = {
-        getOwnPropertyDescriptor: function(target, key) { 
-            getOwnPropertyDescriptorProps.push(key);
-            switch(key) {
-            case "foo":
-                return {
-                    enumerable: true,
-                    configurable: true,
-                    value: 45
-                };
-            case "bar":
-                return  {
-                    get enumerable() {
-                        enumerableCalled = true;
-                        return true;
-                    },
-                    configurable: true,
-                    value: 50
+        };
+
+        let proxy = new Proxy({}, handler);
+        let foo = {};
+        Object.assign(foo, proxy);
+
+        assert(enumerableCalled);
+
+        assert(ownKeys(foo).length === 2);
+        assert(ownKeys(foo)[0] === "foo");
+        assert(ownKeys(foo)[1] === "bar");
+        assert(foo.foo === 20);
+        assert(foo.bar === "bar");
+
+        assert(ownKeysCalled);
+        assert(getOwnPropertyDescriptorProps.length === 3);
+        assert(getOwnPropertyDescriptorProps[0] === "foo");
+        assert(getOwnPropertyDescriptorProps[1] === "bar");
+        assert(getOwnPropertyDescriptorProps[2] === "baz");
+
+        assert(getProps.length === 2);
+        assert(getProps[0] === "foo");
+        assert(getProps[1] === "bar");
+    });
+
+
+    let oldReflect = Reflect;
+    Reflect = null;
+    assert(Reflect === null); // Make sure Object.assign's use of Reflect is safe.
+
+    test(function() {
+        let ownKeysCalled = false;
+        let getOwnPropertyDescriptorProps = [];
+        let getProps = [];
+        let enumerableCalled = false;
+        let handler = {
+            getOwnPropertyDescriptor: function(target, key) {
+                getOwnPropertyDescriptorProps.push(key);
+                switch(key) {
+                case "foo":
+                    return {
+                        enumerable: true,
+                        configurable: true,
+                        value: 45
+                    };
+                case "bar":
+                    return  {
+                        get enumerable() {
+                            enumerableCalled = true;
+                            return true;
+                        },
+                        configurable: true,
+                        value: 50
+                    }
+                case "baz":
+                    return  {
+                        enumerable: false,
+                        configurable: true,
+                        value: 50
+                    }
+                default:
+                    assert(false, "should not be reached.");
+                    break;
                 }
-            case "baz":
-                return  {
-                    enumerable: false,
-                    configurable: true,
-                    value: 50
+            },
+            ownKeys: function(target) {
+                ownKeysCalled = true;
+                return ["foo", "bar", "baz"];
+            },
+            get: function(target, key) {
+                getProps.push(key);
+                switch(key) {
+                case "foo":
+                    return 20;
+                case "bar":
+                    return "bar";
+                default:
+                    assert(false, "should not be reached.");
+                    break;
                 }
-            default:
-                assert(false, "should not be reached.");
-                break;
-            }
-        },
-        ownKeys: function(target) {
-            ownKeysCalled = true;
-            return ["foo", "bar", "baz"];
-        },
-        get: function(target, key) {
-            getProps.push(key);
-            switch(key) {
-            case "foo":
-                return 20;
-            case "bar":
-                return "bar";
-            default:
-                assert(false, "should not be reached.");
-                break;
             }
-        }
-    };
+        };
 
-    let proxy = new Proxy({}, handler);
-    let foo = {};
-    Object.assign(foo, proxy);
+        let proxy = new Proxy({}, handler);
+        let foo = {};
+        Object.assign(foo, proxy);
 
-    assert(enumerableCalled);
+        assert(enumerableCalled);
 
-    assert(oldReflect.ownKeys(foo).length === 2);
-    assert(oldReflect.ownKeys(foo)[0] === "foo");
-    assert(oldReflect.ownKeys(foo)[1] === "bar");
-    assert(foo.foo === 20);
-    assert(foo.bar === "bar");
+        assert(ownKeys(foo).length === 2);
+        assert(ownKeys(foo)[0] === "foo");
+        assert(ownKeys(foo)[1] === "bar");
+        assert(foo.foo === 20);
+        assert(foo.bar === "bar");
 
-    assert(ownKeysCalled);
-    assert(getOwnPropertyDescriptorProps.length === 3);
-    assert(getOwnPropertyDescriptorProps[0] === "foo");
-    assert(getOwnPropertyDescriptorProps[1] === "bar");
-    assert(getOwnPropertyDescriptorProps[2] === "baz");
+        assert(ownKeysCalled);
+        assert(getOwnPropertyDescriptorProps.length === 3);
+        assert(getOwnPropertyDescriptorProps[0] === "foo");
+        assert(getOwnPropertyDescriptorProps[1] === "bar");
+        assert(getOwnPropertyDescriptorProps[2] === "baz");
 
-    assert(getProps.length === 2);
-    assert(getProps[0] === "foo");
-    assert(getProps[1] === "bar");
+        assert(getProps.length === 2);
+        assert(getProps[0] === "foo");
+        assert(getProps[1] === "bar");
 
-});
+    });
+
+    Reflect = oldReflect;
+}
 
+runTests();
+Reflect.ownKeys = function () {};
+Reflect.getOwnPropertyDescriptor = function () {};
+runTests();