[[Put]] should throw if prototype chain contains a readonly property.
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Feb 2012 06:17:40 +0000 (06:17 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Feb 2012 06:17:40 +0000 (06:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79069

Reviewed by Oliver Hunt.

Currently we only check the base of the put, not the prototype chain.
Fold this check in with the test for accessors.

Source/JavaScriptCore:

* runtime/JSObject.cpp:
(JSC::JSObject::put):
    - Updated to test all objects in the propotype chain for readonly properties.
(JSC::JSObject::putDirectAccessor):
(JSC::putDescriptor):
    - Record the presence of readonly properties on the structure.
* runtime/Structure.cpp:
(JSC::Structure::Structure):
    - hasGetterSetterPropertiesExcludingProto expanded to hasReadOnlyOrGetterSetterPropertiesExcludingProto.
* runtime/Structure.h:
(JSC::Structure::hasReadOnlyOrGetterSetterPropertiesExcludingProto):
(JSC::Structure::setHasGetterSetterProperties):
    - hasGetterSetterPropertiesExcludingProto expanded to hasReadOnlyOrGetterSetterPropertiesExcludingProto.
(JSC::Structure::setContainsReadOnlyProperties):
    - Added.

LayoutTests:

* fast/js/Object-defineProperty-expected.txt:
* fast/js/script-tests/Object-defineProperty.js:
(get shouldBeTrue):
    - Added test case.

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

LayoutTests/ChangeLog
LayoutTests/fast/js/Object-defineProperty-expected.txt
LayoutTests/fast/js/script-tests/Object-defineProperty.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/Structure.cpp
Source/JavaScriptCore/runtime/Structure.h

index 0d1dc75..25868ec 100644 (file)
@@ -1,3 +1,18 @@
+2012-02-20  Gavin Barraclough  <barraclough@apple.com>
+
+        [[Put]] should throw if prototype chain contains a readonly property.
+        https://bugs.webkit.org/show_bug.cgi?id=79069
+
+        Reviewed by Oliver Hunt.
+
+        Currently we only check the base of the put, not the prototype chain.
+        Fold this check in with the test for accessors.
+
+        * fast/js/Object-defineProperty-expected.txt:
+        * fast/js/script-tests/Object-defineProperty.js:
+        (get shouldBeTrue):
+            - Added test case.
+
 2012-02-20  Yanbin Zhang  <yanbin.zhang@intel.com>
 
          There is no complete test cases of optional arguments for MediaStream API and PeerConnection
index 89e6cc0..5932471 100644 (file)
@@ -114,6 +114,8 @@ PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1},
 PASS 'use strict'; var o = Object.defineProperty(Object.defineProperty({foo:1}, 'foo', {get:function(){return 42;}, set:function(x){this.result = x;}}), 'foo', {set:undefined}); o.foo = 42; o.result; threw exception TypeError: setting a property that has only a getter.
 PASS 0 in Object.prototype is true
 PASS '0' in Object.prototype is true
+PASS var o = {}; o.readOnly = false; o.readOnly is true
+PASS 'use strict'; var o = {}; o.readOnly = false; o.readOnly threw exception TypeError: Attempted to assign to readonly property..
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 1febb15..34e4a32 100644 (file)
@@ -161,3 +161,10 @@ Object.defineProperty(Object.prototype, 0, {get:function(){ return false; }, con
 shouldBeTrue("0 in Object.prototype");
 shouldBeTrue("'0' in Object.prototype");
 delete Object.prototype[0];
+
+Object.defineProperty(Object.prototype, 'readOnly', {value:true, configurable:true, writable:false})
+shouldBeTrue("var o = {}; o.readOnly = false; o.readOnly");
+shouldThrow("'use strict'; var o = {}; o.readOnly = false; o.readOnly");
+delete Object.prototype.readOnly;
+
+
index fd89e10..6e8dde5 100644 (file)
@@ -1,3 +1,29 @@
+2012-02-20  Gavin Barraclough  <barraclough@apple.com>
+
+        [[Put]] should throw if prototype chain contains a readonly property.
+        https://bugs.webkit.org/show_bug.cgi?id=79069
+
+        Reviewed by Oliver Hunt.
+
+        Currently we only check the base of the put, not the prototype chain.
+        Fold this check in with the test for accessors.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::put):
+            - Updated to test all objects in the propotype chain for readonly properties.
+        (JSC::JSObject::putDirectAccessor):
+        (JSC::putDescriptor):
+            - Record the presence of readonly properties on the structure.
+        * runtime/Structure.cpp:
+        (JSC::Structure::Structure):
+            - hasGetterSetterPropertiesExcludingProto expanded to hasReadOnlyOrGetterSetterPropertiesExcludingProto.
+        * runtime/Structure.h:
+        (JSC::Structure::hasReadOnlyOrGetterSetterPropertiesExcludingProto):
+        (JSC::Structure::setHasGetterSetterProperties):
+            - hasGetterSetterPropertiesExcludingProto expanded to hasReadOnlyOrGetterSetterPropertiesExcludingProto.
+        (JSC::Structure::setContainsReadOnlyProperties):
+            - Added.
+
 2012-02-20  Mark Hahnenberg  <mhahnenberg@apple.com>
 
         Implement fast path for op_new_array in the baseline JIT
index 7fbe5db..5875a5c 100644 (file)
@@ -135,7 +135,7 @@ void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName
     // Check if there are any setters or getters in the prototype chain
     JSValue prototype;
     if (propertyName != exec->propertyNames().underscoreProto) {
-        for (JSObject* obj = thisObject; !obj->structure()->hasGetterSetterPropertiesExcludingProto(); obj = asObject(prototype)) {
+        for (JSObject* obj = thisObject; !obj->structure()->hasReadOnlyOrGetterSetterPropertiesExcludingProto(); obj = asObject(prototype)) {
             prototype = obj->prototype();
             if (prototype.isNull()) {
                 if (!thisObject->putDirectInternal<PutModePut>(globalData, propertyName, value, 0, slot, getJSFunction(value)) && slot.isStrictMode())
@@ -145,16 +145,18 @@ void JSObject::put(JSCell* cell, ExecState* exec, const Identifier& propertyName
         }
     }
 
-    unsigned attributes;
-    JSCell* specificValue;
-    if ((thisObject->structure()->get(globalData, propertyName, attributes, specificValue) != WTF::notFound) && attributes & ReadOnly) {
-        if (slot.isStrictMode())
-            throwError(exec, createTypeError(exec, StrictModeReadonlyPropertyWriteError));
-        return;
-    }
-
     for (JSObject* obj = thisObject; ; obj = asObject(prototype)) {
-        if (JSValue gs = obj->getDirect(globalData, propertyName)) {
+        unsigned attributes;
+        JSCell* specificValue;
+        size_t offset = obj->structure()->get(globalData, propertyName, attributes, specificValue);
+        if (offset != WTF::notFound) {
+            if (attributes & ReadOnly) {
+                if (slot.isStrictMode())
+                    throwError(exec, createTypeError(exec, StrictModeReadonlyPropertyWriteError));
+                return;
+            }
+
+            JSValue gs = obj->getDirectOffset(offset);
             if (gs.isGetterSetter()) {
                 JSObject* setterFunc = asGetterSetter(gs)->setter();        
                 if (!setterFunc) {
@@ -237,6 +239,9 @@ void JSObject::putDirectAccessor(JSGlobalData& globalData, const Identifier& pro
     if (slot.type() != PutPropertySlot::NewProperty)
         setStructure(globalData, Structure::attributeChangeTransition(globalData, structure(), propertyName, attributes));
 
+    if (attributes & ReadOnly)
+        structure()->setContainsReadOnlyProperties();
+
     structure()->setHasGetterSetterProperties(propertyName == globalData.propertyNames->underscoreProto);
 }
 
@@ -619,6 +624,8 @@ static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& p
         else if (oldDescriptor.value())
             newValue = oldDescriptor.value();
         target->putDirect(exec->globalData(), propertyName, newValue, attributes & ~Accessor);
+        if (attributes & ReadOnly)
+            target->structure()->setContainsReadOnlyProperties();
         return true;
     }
     attributes &= ~ReadOnly;
index 2366afd..f292072 100644 (file)
@@ -167,7 +167,7 @@ Structure::Structure(JSGlobalData& globalData, JSGlobalObject* globalObject, JSV
     , m_dictionaryKind(NoneDictionaryKind)
     , m_isPinnedPropertyTable(false)
     , m_hasGetterSetterProperties(false)
-    , m_hasGetterSetterPropertiesExcludingProto(false)
+    , m_hasReadOnlyGetterSetterPropertiesExcludingProto(false)
     , m_hasNonEnumerableProperties(false)
     , m_attributesInPrevious(0)
     , m_specificFunctionThrashCount(0)
@@ -189,7 +189,7 @@ Structure::Structure(JSGlobalData& globalData)
     , m_dictionaryKind(NoneDictionaryKind)
     , m_isPinnedPropertyTable(false)
     , m_hasGetterSetterProperties(false)
-    , m_hasGetterSetterPropertiesExcludingProto(false)
+    , m_hasReadOnlyGetterSetterPropertiesExcludingProto(false)
     , m_hasNonEnumerableProperties(false)
     , m_attributesInPrevious(0)
     , m_specificFunctionThrashCount(0)
@@ -209,7 +209,7 @@ Structure::Structure(JSGlobalData& globalData, const Structure* previous)
     , m_dictionaryKind(previous->m_dictionaryKind)
     , m_isPinnedPropertyTable(false)
     , m_hasGetterSetterProperties(previous->m_hasGetterSetterProperties)
-    , m_hasGetterSetterPropertiesExcludingProto(previous->m_hasGetterSetterPropertiesExcludingProto)
+    , m_hasReadOnlyGetterSetterPropertiesExcludingProto(previous->m_hasReadOnlyGetterSetterPropertiesExcludingProto)
     , m_hasNonEnumerableProperties(previous->m_hasNonEnumerableProperties)
     , m_attributesInPrevious(0)
     , m_specificFunctionThrashCount(previous->m_specificFunctionThrashCount)
index f4207f5..7a2f415 100644 (file)
@@ -145,12 +145,16 @@ namespace JSC {
         }
 
         bool hasGetterSetterProperties() const { return m_hasGetterSetterProperties; }
-        bool hasGetterSetterPropertiesExcludingProto() const { return m_hasGetterSetterPropertiesExcludingProto; }
+        bool hasReadOnlyOrGetterSetterPropertiesExcludingProto() const { return m_hasReadOnlyGetterSetterPropertiesExcludingProto; }
         void setHasGetterSetterProperties(bool is__proto__)
         {
             m_hasGetterSetterProperties = true;
             if (!is__proto__)
-                m_hasGetterSetterPropertiesExcludingProto = true;
+                m_hasReadOnlyGetterSetterPropertiesExcludingProto = true;
+        }
+        void setContainsReadOnlyProperties()
+        {
+            m_hasReadOnlyGetterSetterPropertiesExcludingProto = true;
         }
 
         bool hasNonEnumerableProperties() const { return m_hasNonEnumerableProperties; }
@@ -288,7 +292,7 @@ namespace JSC {
         unsigned m_dictionaryKind : 2;
         bool m_isPinnedPropertyTable : 1;
         bool m_hasGetterSetterProperties : 1;
-        bool m_hasGetterSetterPropertiesExcludingProto : 1;
+        bool m_hasReadOnlyGetterSetterPropertiesExcludingProto : 1;
         bool m_hasNonEnumerableProperties : 1;
         unsigned m_attributesInPrevious : 7;
         unsigned m_specificFunctionThrashCount : 2;