[JSC] RegExp#lastIndex should handle writable attribute when defining in defineOwnPro...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 6 Mar 2016 20:08:28 +0000 (20:08 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 6 Mar 2016 20:08:28 +0000 (20:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155093

Reviewed by Filip Pizlo.

Before this patch, `setLastIndex(ExecState* exec, size_t lastIndex)` always overwrites the existing value
regardless of writable attribute.
And when defining RegExp#lastIndex in defineOwnProperty, we need to define the value first
before making the attribute readonly. After changing the writable attribute, we cannot define the value.

* runtime/RegExpObject.cpp:
(JSC::RegExpObject::defineOwnProperty):
* runtime/RegExpObject.h:
(JSC::RegExpObject::setLastIndex):
* tests/stress/regexp-last-index-writable.js: Added.
(shouldBe):
(shouldThrow):
(regExpLastIndex):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/RegExpObject.cpp
Source/JavaScriptCore/runtime/RegExpObject.h
Source/JavaScriptCore/tests/stress/regexp-last-index-writable.js [new file with mode: 0644]

index be494bc..0c065d9 100644 (file)
@@ -1,3 +1,24 @@
+2016-03-06  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] RegExp#lastIndex should handle writable attribute when defining in defineOwnProperty path
+        https://bugs.webkit.org/show_bug.cgi?id=155093
+
+        Reviewed by Filip Pizlo.
+
+        Before this patch, `setLastIndex(ExecState* exec, size_t lastIndex)` always overwrites the existing value
+        regardless of writable attribute.
+        And when defining RegExp#lastIndex in defineOwnProperty, we need to define the value first
+        before making the attribute readonly. After changing the writable attribute, we cannot define the value.
+
+        * runtime/RegExpObject.cpp:
+        (JSC::RegExpObject::defineOwnProperty):
+        * runtime/RegExpObject.h:
+        (JSC::RegExpObject::setLastIndex):
+        * tests/stress/regexp-last-index-writable.js: Added.
+        (shouldBe):
+        (shouldThrow):
+        (regExpLastIndex):
+
 2016-03-05  Filip Pizlo  <fpizlo@apple.com>
 
         The most aggressive form of RegExpTest/RegExpExec should speculate more aggressively than just cell
index 4fdaeb4..d61ec74 100644 (file)
@@ -127,10 +127,10 @@ bool RegExpObject::defineOwnProperty(JSObject* object, ExecState* exec, Property
                 return reject(exec, shouldThrow, "Attempting to change value of a readonly property.");
             return true;
         }
-        if (descriptor.writablePresent() && !descriptor.writable())
-            regExp->m_lastIndexIsWritable = false;
         if (descriptor.value())
             regExp->setLastIndex(exec, descriptor.value(), false);
+        if (descriptor.writablePresent() && !descriptor.writable())
+            regExp->m_lastIndexIsWritable = false;
         return true;
     }
 
index a626d29..6535c58 100644 (file)
@@ -41,20 +41,25 @@ public:
     void setRegExp(VM& vm, RegExp* r) { m_regExp.set(vm, this, r); }
     RegExp* regExp() const { return m_regExp.get(); }
 
-    void setLastIndex(ExecState* exec, size_t lastIndex)
+    bool setLastIndex(ExecState* exec, size_t lastIndex)
     {
-        m_lastIndex.setWithoutWriteBarrier(jsNumber(lastIndex));
-        if (LIKELY(m_lastIndexIsWritable))
+        if (LIKELY(m_lastIndexIsWritable)) {
             m_lastIndex.setWithoutWriteBarrier(jsNumber(lastIndex));
-        else
-            throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
+            return true;
+        }
+        throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
+        return false;
     }
-    void setLastIndex(ExecState* exec, JSValue lastIndex, bool shouldThrow)
+    bool setLastIndex(ExecState* exec, JSValue lastIndex, bool shouldThrow)
     {
-        if (LIKELY(m_lastIndexIsWritable))
+        if (LIKELY(m_lastIndexIsWritable)) {
             m_lastIndex.set(exec->vm(), this, lastIndex);
-        else if (shouldThrow)
+            return true;
+        }
+
+        if (shouldThrow)
             throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
+        return false;
     }
     JSValue getLastIndex() const
     {
diff --git a/Source/JavaScriptCore/tests/stress/regexp-last-index-writable.js b/Source/JavaScriptCore/tests/stress/regexp-last-index-writable.js
new file mode 100644 (file)
index 0000000..f6808b8
--- /dev/null
@@ -0,0 +1,52 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, message) {
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        error = e;
+    }
+    if (!error)
+        throw new Error("not thrown.");
+    if (String(error) !== message)
+        throw new Error("bad error: " + String(error));
+}
+
+(function regExpLastIndex() {
+    var regexp = new RegExp('Cocoa');
+    regexp.lastIndex = 'Hello';
+    shouldBe(Reflect.get(regexp, 'lastIndex'), 'Hello');
+    regexp.lastIndex = 42;
+    shouldBe(Reflect.get(regexp, 'lastIndex'), 42);
+
+    regexp.lastIndex = "Hello";
+    shouldBe(Reflect.get(regexp, 'lastIndex'), 'Hello');
+
+    shouldBe(Reflect.defineProperty(regexp, 'lastIndex', {
+        value: 42,
+        writable: false
+    }), true);
+    shouldBe(Reflect.get(regexp, 'lastIndex'), 42);
+    shouldThrow(function () {
+        'use strict';
+        regexp.lastIndex = 'NG';
+    }, `TypeError: Attempted to assign to readonly property.`);
+    shouldBe(Reflect.get(regexp, 'lastIndex'), 42);
+
+    shouldThrow(function () {
+        'use strict';
+        regexp.lastIndex = "NG";
+    }, `TypeError: Attempted to assign to readonly property.`);
+
+    shouldThrow(function () {
+        'use strict';
+        Object.defineProperty(regexp, 'lastIndex', {
+            value: 'NG'
+        });
+    }, `TypeError: Attempting to change value of a readonly property.`);
+    shouldBe(Reflect.get(regexp, 'lastIndex'), 42);
+}());