Skip to content

Commit

Permalink
Make sure that a parsererror is thrown whenever malformed JSON comes …
Browse files Browse the repository at this point in the history
…back from a server (so that the Ajax error handler is called). Makes it uniform across browsers that do and don't have JSON.parse support.
  • Loading branch information
jeresig committed Jan 7, 2010
1 parent c14fa51 commit 308d6cd
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 8 deletions.
18 changes: 12 additions & 6 deletions src/ajax.js
Expand Up @@ -570,20 +570,26 @@ jQuery.extend({

// The filter can actually parse the response
if ( typeof data === "string" ) {
// If the type is "script", eval it in global context
if ( type === "script" || !type && ct.indexOf("javascript") >= 0 ) {
jQuery.globalEval( data );
}

// 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, "@")
.replace(/"[^"\\\n\r]*"|true|false|null|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?/g, "]")
.replace(/(?:^|:|,)(?:\s*\[)+/g, ""))) {
data = (new Function("return " + data))();

} else {
data = (new Function("return " + data))();
throw "JSON Syntax Error: " + data;
}

// If the type is "script", eval it in global context
} else if ( type === "script" || !type && ct.indexOf("javascript") >= 0 ) {
jQuery.globalEval( data );
}
}

Expand Down
1 change: 1 addition & 0 deletions test/data/badjson.js
@@ -0,0 +1 @@
{bad: 1}
23 changes: 21 additions & 2 deletions test/unit/ajax.js
Expand Up @@ -341,13 +341,13 @@ test("jQuery.param()", function() {

test("synchronous request", function() {
expect(1);
ok( /^{ "data"/.test( jQuery.ajax({url: url("data/json_obj.js"), async: false}).responseText ), "check returned text" );
ok( /^{ "data"/.test( jQuery.ajax({url: url("data/json_obj.js"), dataType: "text", async: false}).responseText ), "check returned text" );
});

test("synchronous request with callbacks", function() {
expect(2);
var result;
jQuery.ajax({url: url("data/json_obj.js"), async: false, success: function(data) { ok(true, "sucess callback executed"); result = data; } });
jQuery.ajax({url: url("data/json_obj.js"), async: false, dataType: "text", success: function(data) { ok(true, "sucess callback executed"); result = data; } });
ok( /^{ "data"/.test( result ), "check returned text" );
});

Expand Down Expand Up @@ -821,6 +821,25 @@ test("jQuery.ajax() - script, Remote with scheme-less URL", function() {
});
});

test("jQuery.ajax() - malformed JSON", function() {
expect(1);

stop();

jQuery.ajax({
url: "data/badjson.js",
dataType: "json",
success: function(){
ok( false, "Success." );
start();
},
error: function(xhr, msg) {
equals( "parsererror", msg, "A parse error occurred." );
start();
}
});
});

test("jQuery.ajax() - script by content-type", function() {
expect(1);

Expand Down

3 comments on commit 308d6cd

@DBJDBJ
Copy link

@DBJDBJ DBJDBJ commented on 308d6cd Jan 7, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, I feel petty , but I had to note that this line in ajax.js (line: 576 ):

     if ( window.JSON && window.JSON.parse ) {

Should probably be :

     if ( window.JSON && ("function" === typeof window.JSON.parse) ) {

Thanks: DBJ

@DBJDBJ
Copy link

@DBJDBJ DBJDBJ commented on 308d6cd Jan 8, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I get it now ... For all modern browsers you are simply passing the data string to JSON.parse() without any checks whatsoever.
Leaving it to the implementers of it to throw exception or not if wrong JSON string has arrived.
Your logic is to simply "pass the bucket" to window.JSON ... So if one wants to have her JSON working in all browsers, the same one should sort out her JSON, first. This is OK logic, of course. Not entirely jQuery "hand holding" style, but OK.
In this case you are actually throwing an exception, which I think you should do much more in jQuery in general ;o)

@DBJDBJ
Copy link

@DBJDBJ DBJDBJ commented on 308d6cd Jan 11, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My attempt is here (at the bottom)
http://dbj.org/dbj/?p=470
I had a luxury of mulling over this while You are in a 1.4 releasing frenzy. Maybe You will find it usefull therefore ...

Thanks: DBJ

Please sign in to comment.