jooking / closure-library

Automatically exported from code.google.com/p/closure-library
Apache License 2.0
0 stars 0 forks source link

goog.json should rely on the built-in JSON object when it is available #83

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The performance of goog.json could be improved considerably when used in
browsers that support the JSON object (as specified in ES5). I propose
making the following changes to goog.json:

/**
 * @define {boolean} Whether built-ins JSON.parse and JSON.stringify are
 *     available.
 * @const
 */
goog.json.USE_BUILT_IN_JSON_OBJECT = !!(goog.getObjectByName('JSON.parse') &&
    goog.getObjectByName('JSON.stringify'));

goog.json.parse = function(obj) {
  var text = String(obj);
  if (goog.json.USE_BUILT_IN_JSON_OBJECT) {
    return JSON.parse(text);
  } else if (goog.json.isValid_(text)) {
      /** @preserveTry */
      try {
        return eval('(' + text + ')');
      } catch (ex) {
        // Fall through to Error.
      }
    }
  }
  throw Error('Invalid JSON string: ' + text);
};

This may also merit the following change:

goog.json.serialize = function(object) {
  return goog.json.USE_BUILT_IN_JSON_OBJECT ?
      JSON.stringify(object) : new goog.json.Serializer().serialize(object);
};

However that would change the behavior of goog.json.serialize if object has
a toJSON method, such as Date. However, it is unlikely that anyone depends
on that behavior, so it may be safe to change. The JSDoc would need to be
updated accordingly, and goog.json.Serializer would have to be changed to
support toJSON.

Ideally, when compiling for environments that are known to support the JSON
object (such as newer mobile devices), the logic for goog.json.isValid_ and
goog.json.Serializer could be stripped out by specifying "--define
goog.json.USE_BUILT_IN_JSON_OBJECT=true" as a Compiler flag.

Original issue reported on code.google.com by bolinf...@gmail.com on 8 Dec 2009 at 4:36

GoogleCodeExporter commented 9 years ago
FWIW http://kangax.github.com/es5-compat-table/ says that all modern browsers 
now support ES5's JSON object.

Original comment by jeffschi...@google.com on 17 Sep 2010 at 8:43

GoogleCodeExporter commented 9 years ago
It would be nice but it turns out that this is not straightforward to do. The 
main reason is that eval is much more lenient than the built-in JSON parser and 
would break a ton of users.

Original comment by chrishe...@google.com on 23 Apr 2012 at 9:06

GoogleCodeExporter commented 9 years ago
Another reason this would be nice to have, aside from the performance gains, is 
that it would allow Closure's JSON functions to be used with a Content Security 
Policy that disallows eval.

re #2, would it be reasonable to control this with a compile-time flag?

/**
 * Use the native JSON.parse and JSON.stringify, for goog.json.*, if available.
 * @define {boolean}
 */
goog.USE_NATIVE_JSON = false;

Then users who do nothing will continue to get eval(), and those who want the 
native behavior can set the flag at compile time.

Or, there could be two separate functions, parse and nativeParse. Thoughts?

Original comment by tbreisac...@google.com on 9 Aug 2012 at 10:52

GoogleCodeExporter commented 9 years ago
The Chrome Extension Security Policy now disallows inline JavaScript as well as 
"dangerous string-to-JavaScript methods like eval".

https://developer.chrome.com/extensions/contentSecurityPolicy.html

It would be great if this issue could be prioritized to avoid forcing all 
extensions using goog.json.parse() to switch over to JSON.parse().

Original comment by matti...@google.com on 15 Aug 2012 at 10:30

GoogleCodeExporter commented 9 years ago
Sounds like the recommendation for code that needs to use eval is to override 
goog.json.parse. 

goog.json.parse=JSON.parse

Original comment by le...@google.com on 17 Aug 2012 at 4:44