<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[207341] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/207341">207341</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-10-14 08:58:16 -0700 (Fri, 14 Oct 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>JSON.parse should not modify frozen objects.
https://bugs.webkit.org/show_bug.cgi?id=163430
Reviewed by Saam Barati.
JSTests:
* stress/json-parse-on-frozen-object.js: Added.
Source/JavaScriptCore:
The ES6 spec for JSON.parse (https://tc39.github.io/ecma262/#sec-json.parse and
https://tc39.github.io/ecma262/#sec-internalizejsonproperty) states that it uses
CreateDataProperty() (https://tc39.github.io/ecma262/#sec-createdataproperty) to
set values returned by a reviver. The spec for CreateDataPropertyOrThrow states:
"This abstract operation creates a property whose attributes are set to the same
defaults used for properties created by the ECMAScript language assignment
operator. Normally, the property will not already exist. If it does exist and is
not configurable or if O is not extensible, [[DefineOwnProperty]] will return
false."
Note: CreateDataProperty() will not throw a TypeError.
Since the properties of frozen objects are not extensible, not configurable, and
not writeable, JSON.parse should fail to write to any frozen objects. Similarly,
JSON.parse should fail to delete properties in frozen objects.
In JSON.parse(), we previously write to array elements using the form of
putDirectIndex() that uses mode PutDirectIndexLikePutDirect. This makes it so
that the write (i.e. put) is always successful. We've now fixed this to use
PutDirectIndexShouldNotThrow mode instead, which will fail to put the element if
the array is not writeable.
Also changed Walker::walk() to use the version of methodTable() that takes a VM&
since the VM& is already available.
* runtime/JSONObject.cpp:
(JSC::Walker::walk):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJSTestsChangeLog">trunk/JSTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSONObjectcpp">trunk/Source/JavaScriptCore/runtime/JSONObject.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkJSTestsstressjsonparseonfrozenobjectjs">trunk/JSTests/stress/json-parse-on-frozen-object.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (207340 => 207341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog 2016-10-14 15:39:59 UTC (rev 207340)
+++ trunk/JSTests/ChangeLog 2016-10-14 15:58:16 UTC (rev 207341)
</span><span class="lines">@@ -1,3 +1,12 @@
</span><ins>+2016-10-14 Mark Lam <mark.lam@apple.com>
+
+ JSON.parse should not modify frozen objects.
+ https://bugs.webkit.org/show_bug.cgi?id=163430
+
+ Reviewed by Saam Barati.
+
+ * stress/json-parse-on-frozen-object.js: Added.
+
</ins><span class="cx"> 2016-10-14 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> test262: Failure with RegExp.prototype.compile when pattern is undefined
</span></span></pre></div>
<a id="trunkJSTestsstressjsonparseonfrozenobjectjs"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/json-parse-on-frozen-object.js (0 => 207341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/json-parse-on-frozen-object.js (rev 0)
+++ trunk/JSTests/stress/json-parse-on-frozen-object.js 2016-10-14 15:58:16 UTC (rev 207341)
</span><span class="lines">@@ -0,0 +1,75 @@
</span><ins>+//@ runFTLNoCJIT
+
+function shouldEqual(testId, actual, expected) {
+ if (actual != expected) {
+ throw testId + ": ERROR: expect " + expected + ", actual " + actual;
+ }
+}
+
+function frozenArrayReviver(k, v) {
+ if (k === "a") {
+ this.b = Object.freeze(["unmodifiable"]);
+ return v;
+ }
+ if (k === "0")
+ return "modified";
+ return v;
+}
+
+function frozenArrayLikeObjectReviver(k, v) {
+ if (k === "a") {
+ var obj = {};
+ obj[0] = 'unmodifiable';
+ obj.length = 1;
+ this.b = Object.freeze(obj);
+ return v;
+ }
+ if (k === "0")
+ return "modified";
+ return v;
+}
+
+function frozenArrayReviverWithDelete(k, v) {
+ if (k === "a") {
+ this.b = Object.freeze(["unmodifiable"]);
+ return v;
+ }
+ if (k === "0")
+ return undefined;
+ return v;
+}
+
+function frozenArrayLikeObjectReviverWithDelete(k, v) {
+ if (k === "a") {
+ var obj = {};
+ obj[0] = 'unmodifiable';
+ obj.length = 1;
+ this.b = Object.freeze(obj);
+ return v;
+ }
+ if (k === "0")
+ return undefined;
+ return v;
+}
+
+function runTest(testId, reviver, expectedValue, expectedException) {
+ let numIterations = 10000;
+ for (var i = 0; i < numIterations; i++) {
+ var exception = undefined;
+
+ var obj;
+ try {
+ obj = JSON.parse('{ "a": 0, "b": 0 }', reviver);
+ } catch (e) {
+ exception = "" + e;
+ exception = exception.substr(0, 10); // Search for "TypeError:".
+ }
+ shouldEqual(testId, exception, expectedException);
+ shouldEqual(testId, obj.b[0], expectedValue);
+ }
+}
+
+runTest(10000, frozenArrayReviver, "unmodifiable", undefined);
+runTest(10001, frozenArrayLikeObjectReviver, "unmodifiable", undefined);
+runTest(10002, frozenArrayReviverWithDelete, "unmodifiable", undefined);
+runTest(10003, frozenArrayLikeObjectReviverWithDelete, "unmodifiable", undefined);
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (207340 => 207341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-10-14 15:39:59 UTC (rev 207340)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-10-14 15:58:16 UTC (rev 207341)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2016-10-14 Mark Lam <mark.lam@apple.com>
+
+ JSON.parse should not modify frozen objects.
+ https://bugs.webkit.org/show_bug.cgi?id=163430
+
+ Reviewed by Saam Barati.
+
+ The ES6 spec for JSON.parse (https://tc39.github.io/ecma262/#sec-json.parse and
+ https://tc39.github.io/ecma262/#sec-internalizejsonproperty) states that it uses
+ CreateDataProperty() (https://tc39.github.io/ecma262/#sec-createdataproperty) to
+ set values returned by a reviver. The spec for CreateDataPropertyOrThrow states:
+
+ "This abstract operation creates a property whose attributes are set to the same
+ defaults used for properties created by the ECMAScript language assignment
+ operator. Normally, the property will not already exist. If it does exist and is
+ not configurable or if O is not extensible, [[DefineOwnProperty]] will return
+ false."
+
+ Note: CreateDataProperty() will not throw a TypeError.
+
+ Since the properties of frozen objects are not extensible, not configurable, and
+ not writeable, JSON.parse should fail to write to any frozen objects. Similarly,
+ JSON.parse should fail to delete properties in frozen objects.
+
+ In JSON.parse(), we previously write to array elements using the form of
+ putDirectIndex() that uses mode PutDirectIndexLikePutDirect. This makes it so
+ that the write (i.e. put) is always successful. We've now fixed this to use
+ PutDirectIndexShouldNotThrow mode instead, which will fail to put the element if
+ the array is not writeable.
+
+ Also changed Walker::walk() to use the version of methodTable() that takes a VM&
+ since the VM& is already available.
+
+ * runtime/JSONObject.cpp:
+ (JSC::Walker::walk):
+
</ins><span class="cx"> 2016-10-14 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> test262: Failure with RegExp.prototype.compile when pattern is undefined
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSONObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSONObject.cpp (207340 => 207341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSONObject.cpp 2016-10-14 15:39:59 UTC (rev 207340)
+++ trunk/Source/JavaScriptCore/runtime/JSONObject.cpp 2016-10-14 15:58:16 UTC (rev 207341)
</span><span class="lines">@@ -631,7 +631,7 @@
</span><span class="cx"> inValue = array->getIndexQuickly(index);
</span><span class="cx"> else {
</span><span class="cx"> PropertySlot slot(array, PropertySlot::InternalMethodType::Get);
</span><del>- if (array->methodTable()->getOwnPropertySlotByIndex(array, m_exec, index, slot))
</del><ins>+ if (array->methodTable(vm)->getOwnPropertySlotByIndex(array, m_exec, index, slot))
</ins><span class="cx"> inValue = slot.getValue(m_exec, index);
</span><span class="cx"> else
</span><span class="cx"> inValue = jsUndefined();
</span><span class="lines">@@ -649,9 +649,9 @@
</span><span class="cx"> JSArray* array = arrayStack.peek();
</span><span class="cx"> JSValue filteredValue = callReviver(array, jsString(m_exec, String::number(indexStack.last())), outValue);
</span><span class="cx"> if (filteredValue.isUndefined())
</span><del>- array->methodTable()->deletePropertyByIndex(array, m_exec, indexStack.last());
</del><ins>+ array->methodTable(vm)->deletePropertyByIndex(array, m_exec, indexStack.last());
</ins><span class="cx"> else
</span><del>- array->putDirectIndex(m_exec, indexStack.last(), filteredValue);
</del><ins>+ array->putDirectIndex(m_exec, indexStack.last(), filteredValue, 0, PutDirectIndexShouldNotThrow);
</ins><span class="cx"> RETURN_IF_EXCEPTION(scope, JSValue());
</span><span class="cx"> indexStack.last()++;
</span><span class="cx"> goto arrayStartVisitMember;
</span><span class="lines">@@ -667,7 +667,7 @@
</span><span class="cx"> objectStack.push(object);
</span><span class="cx"> indexStack.append(0);
</span><span class="cx"> propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
</span><del>- object->methodTable()->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
</del><ins>+ object->methodTable(vm)->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
</ins><span class="cx"> RETURN_IF_EXCEPTION(scope, JSValue());
</span><span class="cx"> }
</span><span class="cx"> objectStartVisitMember:
</span><span class="lines">@@ -684,7 +684,7 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
</span><del>- if (object->methodTable()->getOwnPropertySlot(object, m_exec, properties[index], slot))
</del><ins>+ if (object->methodTable(vm)->getOwnPropertySlot(object, m_exec, properties[index], slot))
</ins><span class="cx"> inValue = slot.getValue(m_exec, properties[index]);
</span><span class="cx"> else
</span><span class="cx"> inValue = jsUndefined();
</span><span class="lines">@@ -705,9 +705,9 @@
</span><span class="cx"> PutPropertySlot slot(object);
</span><span class="cx"> JSValue filteredValue = callReviver(object, jsString(m_exec, prop.string()), outValue);
</span><span class="cx"> if (filteredValue.isUndefined())
</span><del>- object->methodTable()->deleteProperty(object, m_exec, prop);
</del><ins>+ object->methodTable(vm)->deleteProperty(object, m_exec, prop);
</ins><span class="cx"> else
</span><del>- object->methodTable()->put(object, m_exec, prop, filteredValue, slot);
</del><ins>+ object->methodTable(vm)->put(object, m_exec, prop, filteredValue, slot);
</ins><span class="cx"> RETURN_IF_EXCEPTION(scope, JSValue());
</span><span class="cx"> indexStack.last()++;
</span><span class="cx"> goto objectStartVisitMember;
</span><span class="lines">@@ -731,7 +731,7 @@
</span><span class="cx"> }
</span><span class="cx"> JSObject* finalHolder = constructEmptyObject(m_exec);
</span><span class="cx"> PutPropertySlot slot(finalHolder);
</span><del>- finalHolder->methodTable()->put(finalHolder, m_exec, vm.propertyNames->emptyIdentifier, outValue, slot);
</del><ins>+ finalHolder->methodTable(vm)->put(finalHolder, m_exec, vm.propertyNames->emptyIdentifier, outValue, slot);
</ins><span class="cx"> return callReviver(finalHolder, jsEmptyString(m_exec), outValue);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>