Skip to content

Commit

Permalink
Ajax: Execute JSONP error script responses
Browse files Browse the repository at this point in the history
Issue gh-4379 was meant to be a bug fix but the JSONP case is a bit special:
under the hood it's a script but it simulates JSON responses in an environment
without a CORS setup and sending JSON payloads on error responses is quite
typical there.

This commit makes JSONP error responses still execute the payload. The regular
script error responses continue to be skipped.

Fixes gh-4771
Closes gh-4773

(cherry picked from commit a1e619b)
  • Loading branch information
fras2560 authored and mgol committed Aug 25, 2020
1 parent beea433 commit 3bae54a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,10 @@ jQuery.extend( {
response = ajaxHandleResponses( s, jqXHR, responses );
}

// Use a noop converter for missing script
if ( !isSuccess && jQuery.inArray( "script", s.dataTypes ) > -1 ) {
// Use a noop converter for missing script but not if jsonp
if ( !isSuccess &&
jQuery.inArray( "script", s.dataTypes ) > -1 &&
jQuery.inArray( "json", s.dataTypes ) < 0 ) {
s.converters[ "text script" ] = function() {};
}

Expand Down
13 changes: 13 additions & 0 deletions test/unit/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,19 @@ QUnit.module( "ajax", {
};
} );

ajaxTest( "jQuery.ajax() - do execute scripts if JSONP from unsuccessful responses", 1, function( assert ) {
var testMsg = "Unsuccessful JSONP requests should have a JSON body";
return {
dataType: "jsonp",
url: url( "mock.php?action=errorWithScript" ),
// error is the significant assertion
error: function( xhr ) {
var expected = { "status": 404, "msg": "Not Found" };
assert.deepEqual( xhr.responseJSON, expected, testMsg );
}
};
} );

ajaxTest( "jQuery.ajax() - do not execute scripts from unsuccessful responses (gh-4250)", 11, function( assert ) {
var globalEval = jQuery.globalEval;

Expand Down

0 comments on commit 3bae54a

Please sign in to comment.