Skip to content
Permalink
Browse files

Make sure we do the malformed JSON check for all both JSON.parse and …

…new Function (this helps to create uniformity between browser implementations of JSON.parse - like where Chrome allows some malformed strings. Thanks to DBJDBJ for the heads-up.
  • Loading branch information
jeresig committed Jan 11, 2010
1 parent 23d600c commit 44e6beb10304789044de2c5a58f5bb82e8321636
Showing with 9 additions and 6 deletions.
  1. +9 −6 src/ajax.js
@@ -572,19 +572,22 @@ jQuery.extend({
if ( typeof data === "string" ) {
// Get the JavaScript object, if JSON is used.
if ( type === "json" || !type && ct.indexOf("json") >= 0 ) {
// Try to use the native JSON parser first
if ( window.JSON && window.JSON.parse ) {
data = window.JSON.parse( data );

// Make sure the incoming data is actual JSON
// Logic borrowed from http://json.org/json2.js
} else if (/^[\],:{}\s]*$/.test(data.replace(/\\(?:["\\\/bfnrt]|u[0-9a-fA-F]{4})/g, "@")
if (/^[\],:{}\s]*$/.test(data.replace(/\\(?:["\\\/bfnrt]|u[0-9a-fA-F]{4})/g, "@")
.replace(/"[^"\\\n\r]*"|true|false|null|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?/g, "]")
.replace(/(?:^|:|,)(?:\s*\[)+/g, ""))) {

// Try to use the native JSON parser first
if ( window.JSON && window.JSON.parse ) {
data = window.JSON.parse( data );

} else {
data = (new Function("return " + data))();
}

} else {
throw "JSON.parse";
throw "Invalid JSON: " + data;
}

// If the type is "script", eval it in global context

6 comments on commit 44e6beb

@jaubourg

This comment has been minimized.

Copy link
Member

jaubourg replied Jan 12, 2010

It's too bad we have to penalize browsers with properly validating JSON parsers. We discussed this in #jquery-ot with SuperSlex and he came with this patch (pre-yourse) which I think should be considered:

http://github.com/SlexAxton/jquery/commit/57313eb69a7c87f862440b062932e91c0e162c6c

It uses a support property to determine if data should be controlled or not.

@curiousdannii

This comment has been minimized.

Copy link

curiousdannii replied Jan 12, 2010

These regexs should be cached.

@DBJDBJ

This comment has been minimized.

Copy link

DBJDBJ replied Jan 12, 2010

@curiousdannii

This comment has been minimized.

Copy link

curiousdannii replied Jan 14, 2010

I just realised the regexs don't need to be cached in this situation...

If there are a lot of commercial services using malformed JSON, could there be an ajaxSettings option to disable the strict parsing? They could just as easily get the data as text and parse it manually, but a simple option might be appreciated too.

@DBJDBJ

This comment has been minimized.

Copy link

DBJDBJ replied Jan 15, 2010

I personally would not agree.
I would rather sort out the offending messages (JSON) than make library which will work with illegal message formats(JSON).
As I did yesterday after switching to 1.4, for one (very large) SOA implementation where I (of all people) have done some really stupid JSON ;o(

@DBJDBJ

This comment has been minimized.

Copy link

DBJDBJ replied Jan 15, 2010

John, a slight efficiency improvement perhaps inside ajax.js ? Instead of this :
get: function( url, data, callback, type ) {
// shift arguments if data argument was omited
if ( jQuery.isFunction( data ) ) {
type = type || callback;
callback = data;
data = null;
}

    return jQuery.ajax({
        type: "GET",
        url: url,
        data: data,
        success: callback,
        dataType: type
    });
},

I would do this :
get: function( url, data, callback, type ) {
// shift arguments if data argument was omited
if ( data && jQuery.isFunction( data ) ) { // DBJ: do not call isFunction if data is null
type = type || callback;
callback = data;
data = null;
}
// DBJ: polliticaly correct JSON ;o)
return jQuery.ajax({
"type": "GET",
"url": url,
"data": data,
"success": callback,
"dataType": type
});
},

--DBJ

Please sign in to comment.
You can’t perform that action at this time.