@putByValDirect in Array.of and Array.from overwrites non-writable/configurable prope...
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Jan 2017 23:30:57 +0000 (23:30 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Jan 2017 23:30:57 +0000 (23:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=153486

Reviewed by Saam Barati.

JSTests:

New regression test.

* stress/regress-153486.js: Added.
(shouldEqual):
(makeUnwriteableUnconfigurableObject):
(testArrayOf):
(testArrayFrom):
(runTest):

Source/JavaScriptCore:

Moved read only check in putDirect() to all paths.

* runtime/SparseArrayValueMap.cpp:
(JSC::SparseArrayValueMap::putDirect):

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

JSTests/ChangeLog
JSTests/stress/regress-153486.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp

index 187f20d..456dc97 100644 (file)
@@ -1,3 +1,19 @@
+2017-01-06  Michael Saboff  <msaboff@apple.com>
+
+        @putByValDirect in Array.of and Array.from overwrites non-writable/configurable properties
+        https://bugs.webkit.org/show_bug.cgi?id=153486
+
+        Reviewed by Saam Barati.
+
+        New regression test.
+
+        * stress/regress-153486.js: Added.
+        (shouldEqual):
+        (makeUnwriteableUnconfigurableObject):
+        (testArrayOf):
+        (testArrayFrom):
+        (runTest):
+
 2016-12-30  Filip Pizlo  <fpizlo@apple.com>
 
         DeferGC::~DeferGC should be super cheap
diff --git a/JSTests/stress/regress-153486.js b/JSTests/stress/regress-153486.js
new file mode 100644 (file)
index 0000000..fa68371
--- /dev/null
@@ -0,0 +1,48 @@
+// Check frozen Array.of and Array.from don't modify unwritable/unconfigurable entries.
+
+let totalFailed = 0;
+
+function shouldEqual(testId, iteration, actual, expected) {
+    if (actual != expected) {
+        throw "Test #" + testId + ", iteration " + iteration + ", ERROR: expected \"" + expected + "\", got \"" + actual + "\"";
+    }
+}
+
+function makeUnwriteableUnconfigurableObject()
+{
+    return Object.defineProperty([], 0, {value: "frozen", writable: false, configurable: false});
+}
+
+function testArrayOf(obj)
+{
+    Array.of.call(function() { return obj; }, "no longer frozen");
+}
+
+noInline(testArrayOf);
+
+function testArrayFrom(obj)
+{
+    Array.from.call(function() { return obj; }, ["no longer frozen"]);
+}
+
+noInline(testArrayFrom);
+
+let numIterations = 10000;
+
+function runTest(testId, test, sourceMaker, expectedException) {
+    for (var i = 0; i < numIterations; i++) {
+        var exception = "No exception";
+        var obj = sourceMaker();
+
+        try {
+            test(obj);
+        } catch (e) {
+            exception = "" + e;
+            exception = exception.substr(0, 10); // Search for "TypeError:".
+        }
+        shouldEqual(testId, i, exception, expectedException);
+    }
+}
+
+runTest(1, testArrayOf, makeUnwriteableUnconfigurableObject, "TypeError:");
+runTest(2, testArrayFrom, makeUnwriteableUnconfigurableObject, "TypeError:");
index 5784643..ea063f6 100644 (file)
@@ -1,3 +1,15 @@
+2017-01-06  Michael Saboff  <msaboff@apple.com>
+
+        @putByValDirect in Array.of and Array.from overwrites non-writable/configurable properties
+        https://bugs.webkit.org/show_bug.cgi?id=153486
+
+        Reviewed by Saam Barati.
+
+        Moved read only check in putDirect() to all paths.
+
+        * runtime/SparseArrayValueMap.cpp:
+        (JSC::SparseArrayValueMap::putDirect):
+
 2016-12-30  Filip Pizlo  <fpizlo@apple.com>
 
         DeferGC::~DeferGC should be super cheap
index 10fae9a..daa56ad 100644 (file)
@@ -137,17 +137,17 @@ bool SparseArrayValueMap::putDirect(ExecState* exec, JSObject* array, unsigned i
     AddResult result = add(array, i);
     SparseArrayEntry& entry = result.iterator->value;
 
-    if (mode != PutDirectIndexLikePutDirect && !array->isStructureExtensible()) {
-        // To save a separate find & add, we first always add to the sparse map.
-        // In the uncommon case that this is a new property, and the array is not
-        // extensible, this is not the right thing to have done - so remove again.
-        if (result.isNewEntry) {
-            remove(result.iterator);
-            return typeError(exec, scope, shouldThrow, ASCIILiteral(NonExtensibleObjectPropertyDefineError));
-        }
-        if (entry.attributes & ReadOnly)
-            return typeError(exec, scope, shouldThrow, ASCIILiteral(ReadonlyPropertyWriteError));
+    // To save a separate find & add, we first always add to the sparse map.
+    // In the uncommon case that this is a new property, and the array is not
+    // extensible, this is not the right thing to have done - so remove again.
+    if (mode != PutDirectIndexLikePutDirect && result.isNewEntry && !array->isStructureExtensible()) {
+        remove(result.iterator);
+        return typeError(exec, scope, shouldThrow, ASCIILiteral(NonExtensibleObjectPropertyDefineError));
     }
+
+    if (entry.attributes & ReadOnly)
+        return typeError(exec, scope, shouldThrow, ASCIILiteral(ReadonlyPropertyWriteError));
+
     entry.attributes = attributes;
     entry.set(vm, this, value);
     return true;