<!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>[47857] 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/47857">47857</a></dd>
<dt>Author</dt> <dd>oliver@apple.com</dd>
<dt>Date</dt> <dd>2009-08-27 23:17:36 -0700 (Thu, 27 Aug 2009)</dd>
</dl>

<h3>Log Message</h3>
<pre>JSON Stringifier does not follow ES5 spec for handling of Number, String and Boolean objects
https://bugs.webkit.org/show_bug.cgi?id=28797

Reviewed by Maciej Stachowiak

Fixed unwrapBoxedPrimitive to do the right thing, which necessitated a couple of new exception
checks, and corrected the logic in gap to correctly convert Number and String objects.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJavaScriptCoreChangeLog">trunk/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkJavaScriptCoreruntimeJSONObjectcpp">trunk/JavaScriptCore/runtime/JSONObject.cpp</a></li>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastjsJSONstringifyexpectedtxt">trunk/LayoutTests/fast/js/JSON-stringify-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastjsresourcesJSONstringifyjs">trunk/LayoutTests/fast/js/resources/JSON-stringify.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/ChangeLog (47856 => 47857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/ChangeLog        2009-08-28 04:54:31 UTC (rev 47856)
+++ trunk/JavaScriptCore/ChangeLog        2009-08-28 06:17:36 UTC (rev 47857)
</span><span class="lines">@@ -1,5 +1,21 @@
</span><span class="cx"> 2009-08-27  Oliver Hunt  &lt;oliver@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        Reviewed by Maciej Stachowiak.
+
+        JSON Stringifier does not follow ES5 spec for handling of Number, String and Boolean objects
+        https://bugs.webkit.org/show_bug.cgi?id=28797
+
+        Fixed unwrapBoxedPrimitive to do the right thing, which necessitated a couple of new exception
+        checks, and corrected the logic in gap to correctly convert Number and String objects.
+
+        * runtime/JSONObject.cpp:
+        (JSC::unwrapBoxedPrimitive):
+        (JSC::gap):
+        (JSC::Stringifier::Stringifier):
+        (JSC::Stringifier::appendStringifiedValue):
+
+2009-08-27  Oliver Hunt  &lt;oliver@apple.com&gt;
+
</ins><span class="cx">         Reviewed by Adam Roben.
</span><span class="cx"> 
</span><span class="cx">         JSON.stringify replacer array does not accept values that are not string primitives.
</span></span></pre></div>
<a id="trunkJavaScriptCoreruntimeJSONObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/runtime/JSONObject.cpp (47856 => 47857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/runtime/JSONObject.cpp        2009-08-28 04:54:31 UTC (rev 47856)
+++ trunk/JavaScriptCore/runtime/JSONObject.cpp        2009-08-28 06:17:36 UTC (rev 47857)
</span><span class="lines">@@ -120,38 +120,47 @@
</span><span class="cx"> 
</span><span class="cx"> // ------------------------------ helper functions --------------------------------
</span><span class="cx"> 
</span><del>-static inline JSValue unwrapBoxedPrimitive(JSValue value)
</del><ins>+static inline JSValue unwrapBoxedPrimitive(ExecState* exec, JSValue value)
</ins><span class="cx"> {
</span><span class="cx">     if (!value.isObject())
</span><span class="cx">         return value;
</span><del>-    if (!asObject(value)-&gt;inherits(&amp;NumberObject::info) &amp;&amp; !asObject(value)-&gt;inherits(&amp;StringObject::info) &amp;&amp; !asObject(value)-&gt;inherits(&amp;BooleanObject::info))
-        return value;
-    return static_cast&lt;JSWrapperObject*&gt;(asObject(value))-&gt;internalValue();
</del><ins>+    JSObject* object = asObject(value);
+    if (object-&gt;inherits(&amp;NumberObject::info))
+        return jsNumber(exec, object-&gt;toNumber(exec));
+    if (object-&gt;inherits(&amp;StringObject::info))
+        return jsString(exec, object-&gt;toString(exec));
+    if (object-&gt;inherits(&amp;BooleanObject::info))
+        return object-&gt;toPrimitive(exec);
+    return value;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-static inline UString gap(JSValue space)
</del><ins>+static inline UString gap(ExecState* exec, JSValue space)
</ins><span class="cx"> {
</span><del>-    space = unwrapBoxedPrimitive(space);
</del><ins>+    const int maxGapLength = 10;
+    space = unwrapBoxedPrimitive(exec, space);
</ins><span class="cx"> 
</span><span class="cx">     // If the space value is a number, create a gap string with that number of spaces.
</span><span class="cx">     double spaceCount;
</span><span class="cx">     if (space.getNumber(spaceCount)) {
</span><del>-        const int maxSpaceCount = 100;
</del><span class="cx">         int count;
</span><del>-        if (spaceCount &gt; maxSpaceCount)
-            count = maxSpaceCount;
</del><ins>+        if (spaceCount &gt; maxGapLength)
+            count = maxGapLength;
</ins><span class="cx">         else if (!(spaceCount &gt; 0))
</span><span class="cx">             count = 0;
</span><span class="cx">         else
</span><span class="cx">             count = static_cast&lt;int&gt;(spaceCount);
</span><del>-        UChar spaces[maxSpaceCount];
</del><ins>+        UChar spaces[maxGapLength];
</ins><span class="cx">         for (int i = 0; i &lt; count; ++i)
</span><span class="cx">             spaces[i] = ' ';
</span><span class="cx">         return UString(spaces, count);
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     // If the space value is a string, use it as the gap string, otherwise use no gap string.
</span><del>-    return space.getString();
</del><ins>+    UString spaces = space.getString();
+    if (spaces.size() &gt; maxGapLength) {
+        spaces = spaces.substr(0, maxGapLength);
+    }
+    return spaces;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> // ------------------------------ PropertyNameForFunctionCall --------------------------------
</span><span class="lines">@@ -187,7 +196,7 @@
</span><span class="cx">     , m_usingArrayReplacer(false)
</span><span class="cx">     , m_arrayReplacerPropertyNames(exec)
</span><span class="cx">     , m_replacerCallType(CallTypeNone)
</span><del>-    , m_gap(gap(space))
</del><ins>+    , m_gap(gap(exec, space))
</ins><span class="cx"> {
</span><span class="cx">     exec-&gt;globalData().firstStringifierToMark = this;
</span><span class="cx"> 
</span><span class="lines">@@ -203,12 +212,6 @@
</span><span class="cx">             if (exec-&gt;hadException())
</span><span class="cx">                 break;
</span><span class="cx"> 
</span><del>-            if (name.isObject()) {
-                if (!asObject(name)-&gt;inherits(&amp;NumberObject::info) &amp;&amp; !asObject(name)-&gt;inherits(&amp;StringObject::info))
-                    continue;
-                name = static_cast&lt;JSWrapperObject*&gt;(asObject(name))-&gt;internalValue();
-            }
-
</del><span class="cx">             UString propertyName;
</span><span class="cx">             if (name.getString(propertyName)) {
</span><span class="cx">                 m_arrayReplacerPropertyNames.add(Identifier(exec, propertyName));
</span><span class="lines">@@ -221,8 +224,14 @@
</span><span class="cx">                 continue;
</span><span class="cx">             }
</span><span class="cx"> 
</span><del>-            if (exec-&gt;hadException())
-                return;
</del><ins>+            if (name.isObject()) {
+                if (!asObject(name)-&gt;inherits(&amp;NumberObject::info) &amp;&amp; !asObject(name)-&gt;inherits(&amp;StringObject::info))
+                    continue;
+                propertyName = name.toString(exec);
+                if (exec-&gt;hadException())
+                    break;
+                m_arrayReplacerPropertyNames.add(Identifier(exec, propertyName));
+            }
</ins><span class="cx">         }
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="lines">@@ -369,8 +378,11 @@
</span><span class="cx">         return StringifySucceeded;
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    value = unwrapBoxedPrimitive(value);
</del><ins>+    value = unwrapBoxedPrimitive(m_exec, value);
</ins><span class="cx"> 
</span><ins>+    if (m_exec-&gt;hadException())
+        return StringifyFailed;
+
</ins><span class="cx">     if (value.isBoolean()) {
</span><span class="cx">         builder.append(value.getBoolean() ? &quot;true&quot; : &quot;false&quot;);
</span><span class="cx">         return StringifySucceeded;
</span></span></pre></div>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (47856 => 47857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2009-08-28 04:54:31 UTC (rev 47856)
+++ trunk/LayoutTests/ChangeLog        2009-08-28 06:17:36 UTC (rev 47857)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2009-08-27  Oliver Hunt  &lt;oliver@apple.com&gt;
+
+        Reviewed by Maciej Stachowiak.
+
+        JSON Stringifier does not follow ES5 spec for handling of Number, String and Boolean objects
+        https://bugs.webkit.org/show_bug.cgi?id=28797
+
+        Added new tests to cover type conversions
+
+        * fast/js/JSON-stringify-expected.txt:
+        * fast/js/resources/JSON-stringify.js:
+        (createTests):
+
</ins><span class="cx"> 2009-08-27  Drew Wilson  &lt;atwilson@google.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Unreviewed.
</span></span></pre></div>
<a id="trunkLayoutTestsfastjsJSONstringifyexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/js/JSON-stringify-expected.txt (47856 => 47857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/JSON-stringify-expected.txt        2009-08-28 04:54:31 UTC (rev 47856)
+++ trunk/LayoutTests/fast/js/JSON-stringify-expected.txt        2009-08-28 06:17:36 UTC (rev 47857)
</span><span class="lines">@@ -51,10 +51,18 @@
</span><span class="cx">     }
</span><span class="cx"> PASS tests[i](nativeJSON) is tests[i](JSON)
</span><span class="cx"> function (jsonObject) {
</span><del>-        return jsonObject.stringify(new Boolean(false));
</del><ins>+        var value = new Number(1);
+        value.valueOf = function() { return 2; }
+        return jsonObject.stringify(value);
</ins><span class="cx">     }
</span><del>-PASS tests[i](nativeJSON) is tests[i](JSON)
</del><ins>+PASS tests[i](nativeJSON) is tests[i].expected
</ins><span class="cx"> function (jsonObject) {
</span><ins>+        var value = new Boolean(true);
+        value.valueOf = function() { return 2; }
+        return jsonObject.stringify(value);
+    }
+PASS tests[i](nativeJSON) is tests[i].expected
+function (jsonObject) {
</ins><span class="cx">         return jsonObject.stringify(true);
</span><span class="cx">     }
</span><span class="cx"> PASS tests[i](nativeJSON) is tests[i](JSON)
</span><span class="lines">@@ -199,6 +207,22 @@
</span><span class="cx">     }
</span><span class="cx"> PASS tests[i](nativeJSON) is tests[i](JSON)
</span><span class="cx"> function (jsonObject) {
</span><ins>+        return jsonObject.stringify(simpleObject, null, 10);
+    }
+PASS tests[i](nativeJSON) is tests[i](JSON)
+function (jsonObject) {
+        return jsonObject.stringify(simpleObject, null, 11);
+    }
+PASS tests[i](nativeJSON) is tests[i].expected
+function (jsonObject) {
+        return jsonObject.stringify(simpleObject, null, &quot;          &quot;);
+    }
+PASS tests[i](nativeJSON) is tests[i].expected
+function (jsonObject) {
+        return jsonObject.stringify(simpleObject, null, &quot;           &quot;);
+    }
+PASS tests[i](nativeJSON) is tests[i].expected
+function (jsonObject) {
</ins><span class="cx">         return jsonObject.stringify(complexArray, null, &quot;  &quot;);
</span><span class="cx">     }
</span><span class="cx"> PASS tests[i](nativeJSON) is tests[i](JSON)
</span></span></pre></div>
<a id="trunkLayoutTestsfastjsresourcesJSONstringifyjs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/js/resources/JSON-stringify.js (47856 => 47857)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/resources/JSON-stringify.js        2009-08-28 04:54:31 UTC (rev 47856)
+++ trunk/LayoutTests/fast/js/resources/JSON-stringify.js        2009-08-28 06:17:36 UTC (rev 47857)
</span><span class="lines">@@ -45,9 +45,18 @@
</span><span class="cx">         return jsonObject.stringify(new Boolean(true));
</span><span class="cx">     });
</span><span class="cx">     result.push(function(jsonObject){
</span><del>-        return jsonObject.stringify(new Boolean(false));
</del><ins>+        var value = new Number(1);
+        value.valueOf = function() { return 2; }
+        return jsonObject.stringify(value);
</ins><span class="cx">     });
</span><ins>+    result[result.length - 1].expected = '2';
</ins><span class="cx">     result.push(function(jsonObject){
</span><ins>+        var value = new Boolean(true);
+        value.valueOf = function() { return 2; }
+        return jsonObject.stringify(value);
+    });
+    result[result.length - 1].expected = '2';
+    result.push(function(jsonObject){
</ins><span class="cx">         return jsonObject.stringify(true);
</span><span class="cx">     });
</span><span class="cx">     result.push(function(jsonObject){
</span><span class="lines">@@ -167,6 +176,21 @@
</span><span class="cx">         return jsonObject.stringify(simpleObject, null, 4);
</span><span class="cx">     });
</span><span class="cx">     result.push(function(jsonObject){
</span><ins>+        return jsonObject.stringify(simpleObject, null, 10);
+    });
+    result.push(function(jsonObject){
+        return jsonObject.stringify(simpleObject, null, 11);
+    });
+    result[result.length - 1].expected = JSON.stringify(simpleObject, null, 10);
+    result.push(function(jsonObject){
+        return jsonObject.stringify(simpleObject, null, &quot;          &quot;);
+    });
+    result[result.length - 1].expected = JSON.stringify(simpleObject, null, 10);
+    result.push(function(jsonObject){
+        return jsonObject.stringify(simpleObject, null, &quot;           &quot;);
+    });
+    result[result.length - 1].expected = JSON.stringify(simpleObject, null, 10);
+    result.push(function(jsonObject){
</ins><span class="cx">         return jsonObject.stringify(complexArray, null, &quot;  &quot;);
</span><span class="cx">     });
</span><span class="cx">     result.push(function(jsonObject){
</span><span class="lines">@@ -371,9 +395,9 @@
</span><span class="cx">         if (tests[i].throws)
</span><span class="cx">             shouldThrow('tests[i](nativeJSON)');
</span><span class="cx">         else if (tests[i].expected)
</span><del>-            shouldBe('tests[i](nativeJSON)',  'tests[i].expected')
</del><ins>+            shouldBe('tests[i](nativeJSON)',  &quot;tests[i].expected&quot;);
</ins><span class="cx">         else
</span><del>-            shouldBe('tests[i](nativeJSON)',  'tests[i](JSON)');
</del><ins>+            shouldBe('tests[i](nativeJSON)',  &quot;tests[i](JSON)&quot;);
</ins><span class="cx">     }catch(e){}
</span><span class="cx"> }
</span><span class="cx"> successfullyParsed = true;
</span></span></pre>
</div>
</div>

</body>
</html>