Bug 557371 - Make JSON.stringify behavior on Boolean, String, and Number objects to-spec. r=jorendorff
authorJeff Walden <jwalden@mit.edu>
Fri, 31 Dec 2010 11:41:23 -0600
changeset 64212 c7e3c056478be7550bd39a37c1c3b96c93c77337
parent 64211 423a8e7df45516bd327f8d4a75c49b9b9b1b2c06
child 64213 6179a5b48142536587afc6751c50a90b79eaeb43
push id19328
push usercleary@mozilla.com
push dateTue, 29 Mar 2011 21:56:11 +0000
treeherdermozilla-central@3d0784802ce6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs557371
milestone2.0b13pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 557371 - Make JSON.stringify behavior on Boolean, String, and Number objects to-spec. r=jorendorff
dom/src/json/test/unit/test_encode.js
dom/src/json/test/unit/test_wrappers.js
js/src/jsnum.cpp
js/src/json.cpp
js/src/jsstr.cpp
js/src/tests/ecma_5/JSON/jstests.list
js/src/tests/ecma_5/JSON/stringify-boxed-primitives.js
--- a/dom/src/json/test/unit/test_encode.js
+++ b/dom/src/json/test/unit/test_encode.js
@@ -99,23 +99,17 @@ function getTestPairs() {
 
 function testStringEncode() {
   // test empty arg
   do_check_eq(null, nativeJSON.encode());
 
   var pairs = getTestPairs();
   for each(pair in pairs) {
     var nativeResult = nativeJSON.encode(pair[1]);
-    var crockfordResult = crockfordJSON.stringify(pair[1]);
     do_check_eq(pair[0], nativeResult);
-    
-    // Don't follow json2.js handling of non-objects
-    if (pair[1] && (typeof pair[1] == "object")) {
-      do_check_eq(crockfordResult, nativeResult);
-    }
   }
 }
 
 function testOutputStreams() {
   function writeToFile(obj, charset, writeBOM) {
     var jsonFile = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsILocalFile);
     jsonFile.initWithFile(outputDir);
     jsonFile.append("test.json");
--- a/dom/src/json/test/unit/test_wrappers.js
+++ b/dom/src/json/test/unit/test_wrappers.js
@@ -90,19 +90,17 @@ function getTestPairs() {
   return testPairs;
 }
 
 function testStringEncode() {
   var pairs = getTestPairs();
   for each(pair in pairs) {
     print(pair)
     var nativeResult = JSON.stringify(pair[1]);
-    var crockfordResult = crockfordJSON.stringify(pair[1]);
     do_check_eq(pair[0], nativeResult);
-    do_check_eq(crockfordResult, nativeResult);
   }
 }
 
 function decode_strings() {
   // empty object
   var x = JSON.parse("{}");
   do_check_eq(typeof x, "object");
 
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -939,17 +939,16 @@ JS_DEFINE_TRCINFO_2(num_toString,
 
 static JSFunctionSpec number_methods[] = {
 #if JS_HAS_TOSOURCE
     JS_FN(js_toSource_str,       num_toSource,          0, 0),
 #endif
     JS_TN(js_toString_str,       num_toString,          1, 0, &num_toString_trcinfo),
     JS_FN(js_toLocaleString_str, num_toLocaleString,    0, 0),
     JS_FN(js_valueOf_str,        js_num_valueOf,        0, 0),
-    JS_FN(js_toJSON_str,         js_num_valueOf,        0, 0),
     JS_FN("toFixed",             num_toFixed,           1, 0),
     JS_FN("toExponential",       num_toExponential,     1, 0),
     JS_FN("toPrecision",         num_toPrecision,       1, 0),
     JS_FS_END
 };
 
 /* NB: Keep this in synch with number_constants[]. */
 enum nc_slot {
--- a/js/src/json.cpp
+++ b/js/src/json.cpp
@@ -489,73 +489,98 @@ CallReplacerFunction(JSContext *cx, jsid
     }
 
     return JS_TRUE;
 }
 
 static JSBool
 Str(JSContext *cx, jsid id, JSObject *holder, StringifyContext *scx, Value *vp, bool callReplacer)
 {
-    JS_CHECK_RECURSION(cx, return JS_FALSE);
+    JS_CHECK_RECURSION(cx, return false);
 
-    if (vp->isObject() && !js_TryJSON(cx, vp))
-        return JS_FALSE;
+    /*
+     * This method implements the Str algorithm in ES5 15.12.3, but we move
+     * property retrieval (step 1) into callers to stream the stringification
+     * process and avoid constantly copying strings.
+     */
 
+    /* Step 2. */
+    if (vp->isObject() && !js_TryJSON(cx, vp))
+        return false;
+
+    /* Step 3. */
     if (callReplacer && !CallReplacerFunction(cx, id, holder, scx, vp))
-        return JS_FALSE;
+        return false;
 
-    // catches string and number objects with no toJSON
+    /* Step 4. */
     if (vp->isObject()) {
         JSObject *obj = &vp->toObject();
         Class *clasp = obj->getClass();
-        if (clasp == &js_StringClass || clasp == &js_NumberClass)
+        if (clasp == &js_NumberClass) {
+            double d;
+            if (!ValueToNumber(cx, *vp, &d))
+                return false;
+            vp->setNumber(d);
+        } else if (clasp == &js_StringClass) {
+            JSString *str = js_ValueToString(cx, *vp);
+            if (!str)
+                return false;
+            vp->setString(str);
+        } else if (clasp == &js_BooleanClass) {
             *vp = obj->getPrimitiveThis();
+        }
     }
 
+    /* Step 8. */
     if (vp->isString()) {
         JSString *str = vp->toString();
         size_t length = str->length();
         const jschar *chars = str->getChars(cx);
         if (!chars)
-            return JS_FALSE;
+            return false;
         return write_string(cx, scx->sb, chars, length);
     }
 
+    /* Step 5. */
     if (vp->isNull())
         return scx->sb.append("null");
 
+    /* Steps 6-7. */
     if (vp->isBoolean())
         return vp->toBoolean() ? scx->sb.append("true") : scx->sb.append("false");
 
+    /* Step 9. */
     if (vp->isNumber()) {
         if (vp->isDouble()) {
             jsdouble d = vp->toDouble();
             if (!JSDOUBLE_IS_FINITE(d))
                 return scx->sb.append("null");
         }
 
         StringBuffer sb(cx);
         if (!NumberValueToStringBuffer(cx, *vp, sb))
-            return JS_FALSE;
+            return false;
 
         return scx->sb.append(sb.begin(), sb.length());
     }
 
+    /* Step 10. */
     if (vp->isObject() && !IsFunctionObject(*vp) && !IsXML(*vp)) {
         JSBool ok;
 
         scx->depth++;
         ok = (JS_IsArrayObject(cx, &vp->toObject()) ? JA : JO)(cx, vp, scx);
         scx->depth--;
 
         return ok;
     }
 
+    /* Step 11. */
     vp->setUndefined();
-    return JS_TRUE;
+    return true;
 }
 
 JSBool
 js_Stringify(JSContext *cx, Value *vp, JSObject *replacer, const Value &space,
              StringBuffer &sb)
 {
     StringifyContext scx(cx, sb, replacer);
     if (!scx.initializeGap(cx, space) || !scx.initializeStack())
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3068,17 +3068,16 @@ static JSFunctionSpec string_methods[] =
 #if JS_HAS_TOSOURCE
     JS_FN("quote",             str_quote,             0,JSFUN_GENERIC_NATIVE),
     JS_FN(js_toSource_str,     str_toSource,          0,0),
 #endif
 
     /* Java-like methods. */
     JS_FN(js_toString_str,     js_str_toString,       0,0),
     JS_FN(js_valueOf_str,      js_str_toString,       0,0),
-    JS_FN(js_toJSON_str,       js_str_toString,       0,0),
     JS_FN("substring",         str_substring,         2,JSFUN_GENERIC_NATIVE),
     JS_FN("toLowerCase",       str_toLowerCase,       0,JSFUN_GENERIC_NATIVE),
     JS_FN("toUpperCase",       str_toUpperCase,       0,JSFUN_GENERIC_NATIVE),
     JS_FN("charAt",            js_str_charAt,         1,JSFUN_GENERIC_NATIVE),
     JS_FN("charCodeAt",        js_str_charCodeAt,     1,JSFUN_GENERIC_NATIVE),
     JS_FN("indexOf",           str_indexOf,           1,JSFUN_GENERIC_NATIVE),
     JS_FN("lastIndexOf",       str_lastIndexOf,       1,JSFUN_GENERIC_NATIVE),
     JS_FN("trim",              str_trim,              0,JSFUN_GENERIC_NATIVE),
--- a/js/src/tests/ecma_5/JSON/jstests.list
+++ b/js/src/tests/ecma_5/JSON/jstests.list
@@ -1,5 +1,6 @@
 url-prefix ../../jsreftest.html?test=ecma_5/JSON/
 script cyclic-stringify.js
 script small-codepoints.js
 script trailing-comma.js
 script stringify-gap.js
+script stringify-boxed-primitives.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/JSON/stringify-boxed-primitives.js
@@ -0,0 +1,127 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+
+var gTestfile = 'stringify-boxed-primitives.js';
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 584909;
+var summary = "Stringification of Boolean/String/Number objects";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function redefine(obj, prop, fun)
+{
+  var desc =
+    { value: fun, writable: true, configurable: true, enumerable: false };
+  Object.defineProperty(obj, prop, desc);
+}
+
+assertEq(JSON.stringify(new Boolean(false)), "false");
+
+assertEq(JSON.stringify(new Number(5)), "5");
+
+assertEq(JSON.stringify(new String("foopy")), '"foopy"');
+
+
+var numToString = Number.prototype.toString;
+var numValueOf = Number.prototype.valueOf;
+var objToString = Object.prototype.toString;
+var objValueOf = Object.prototype.valueOf;
+var boolToString = Boolean.prototype.toString;
+var boolValueOf = Boolean.prototype.valueOf;
+
+redefine(Boolean.prototype, "toString", function() { return 17; });
+assertEq(JSON.stringify(new Boolean(false)), "false")
+delete Boolean.prototype.toString;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+delete Object.prototype.toString;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+delete Boolean.prototype.valueOf;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+delete Object.prototype.valueOf;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+
+
+redefine(Boolean.prototype, "toString", boolToString);
+redefine(Boolean.prototype, "valueOf", boolValueOf);
+redefine(Object.prototype, "toString", objToString);
+redefine(Object.prototype, "valueOf", objValueOf);
+
+redefine(Number.prototype, "toString", function() { return 42; });
+assertEq(JSON.stringify(new Number(5)), "5");
+redefine(Number.prototype, "valueOf", function() { return 17; });
+assertEq(JSON.stringify(new Number(5)), "17");
+delete Number.prototype.toString;
+assertEq(JSON.stringify(new Number(5)), "17");
+delete Number.prototype.valueOf;
+assertEq(JSON.stringify(new Number(5)), "null"); // isNaN(Number("[object Number]"))
+delete Object.prototype.toString;
+try
+{
+  JSON.stringify(new Number(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToNumber failure, should throw TypeError");
+}
+delete Object.prototype.valueOf;
+try
+{
+  JSON.stringify(new Number(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToNumber failure, should throw TypeError");
+}
+
+
+redefine(Number.prototype, "toString", numToString);
+redefine(Number.prototype, "valueOf", numValueOf);
+redefine(Object.prototype, "toString", objToString);
+redefine(Object.prototype, "valueOf", objValueOf);
+
+
+redefine(String.prototype, "valueOf", function() { return 17; });
+assertEq(JSON.stringify(new String(5)), '"5"');
+redefine(String.prototype, "toString", function() { return 42; });
+assertEq(JSON.stringify(new String(5)), '"42"');
+delete String.prototype.toString;
+assertEq(JSON.stringify(new String(5)), '"[object String]"');
+delete Object.prototype.toString;
+assertEq(JSON.stringify(new String(5)), '"17"');
+delete String.prototype.valueOf;
+try
+{
+  JSON.stringify(new String(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToString failure, should throw TypeError");
+}
+delete Object.prototype.valueOf;
+try
+{
+  JSON.stringify(new String(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToString failure, should throw TypeError");
+}
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("All tests passed!");