Skip to content

Commit b29bbd0

Browse files
mgolmgol
committed
Ajax: Drop the json to jsonp auto-promotion logic
Previously, `jQuery.ajax` with `dataType: 'json'` with a provided callback was automatically converted to a jsonp request unless one also specified `jsonp: false`. Today the preferred way of interacting with a cross-domain backend is CORS which works in all browsers jQuery 4 will support. Auto-promoting JSON requests to JSONP ones introduces a security issue as the developer may be unaware they're not just downloading data but executing code from a remote domain. This commit disables the auto-promoting logic. BREAKING CHANGE: to trigger a JSONP request, it's now required to specify `dataType: "jsonp"`; previously some requests with `dataType: "json"` were auto-promoted to JSONP. Fixes gh-3376
1 parent b502866 commit b29bbd0

File tree

5 files changed

+95
-52
lines changed

5 files changed

+95
-52
lines changed

‎src/ajax/jsonp.js‎

Lines changed: 48 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jQuery.ajaxSetup( {
1818
} );
1919

2020
// Detect, normalize options and install callbacks for jsonp requests
21-
jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, jqXHR ) {
21+
jQuery.ajaxPrefilter( "jsonp", function( s, originalSettings, jqXHR ) {
2222

2323
var callbackName, overwritten, responseContainer,
2424
jsonProp = s.jsonp !== false && ( rjsonp.test( s.url ) ?
@@ -29,69 +29,65 @@ jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, jqXHR ) {
2929
rjsonp.test( s.data ) && "data"
3030
);
3131

32-
// Handle iff the expected data type is "jsonp" or we have a parameter to set
33-
if ( jsonProp || s.dataTypes[ 0 ] === "jsonp" ) {
32+
// Get callback name, remembering preexisting value associated with it
33+
callbackName = s.jsonpCallback = typeof s.jsonpCallback === "function" ?
34+
s.jsonpCallback() :
35+
s.jsonpCallback;
3436

35-
// Get callback name, remembering preexisting value associated with it
36-
callbackName = s.jsonpCallback = typeof s.jsonpCallback === "function" ?
37-
s.jsonpCallback() :
38-
s.jsonpCallback;
37+
// Insert callback into url or form data
38+
if ( jsonProp ) {
39+
s[ jsonProp ] = s[ jsonProp ].replace( rjsonp, "$1" + callbackName );
40+
} else if ( s.jsonp !== false ) {
41+
s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.jsonp + "=" + callbackName;
42+
}
3943

40-
// Insert callback into url or form data
41-
if ( jsonProp ) {
42-
s[ jsonProp ] = s[ jsonProp ].replace( rjsonp, "$1" + callbackName );
43-
} else if ( s.jsonp !== false ) {
44-
s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.jsonp + "=" + callbackName;
44+
// Use data converter to retrieve json after script execution
45+
s.converters[ "script json" ] = function() {
46+
if ( !responseContainer ) {
47+
jQuery.error( callbackName + " was not called" );
4548
}
49+
return responseContainer[ 0 ];
50+
};
4651

47-
// Use data converter to retrieve json after script execution
48-
s.converters[ "script json" ] = function() {
49-
if ( !responseContainer ) {
50-
jQuery.error( callbackName + " was not called" );
51-
}
52-
return responseContainer[ 0 ];
53-
};
54-
55-
// Force json dataType
56-
s.dataTypes[ 0 ] = "json";
52+
// Force json dataType
53+
s.dataTypes[ 0 ] = "json";
5754

58-
// Install callback
59-
overwritten = window[ callbackName ];
60-
window[ callbackName ] = function() {
61-
responseContainer = arguments;
62-
};
55+
// Install callback
56+
overwritten = window[ callbackName ];
57+
window[ callbackName ] = function() {
58+
responseContainer = arguments;
59+
};
6360

64-
// Clean-up function (fires after converters)
65-
jqXHR.always( function() {
61+
// Clean-up function (fires after converters)
62+
jqXHR.always( function() {
6663

67-
// If previous value didn't exist - remove it
68-
if ( overwritten === undefined ) {
69-
jQuery( window ).removeProp( callbackName );
64+
// If previous value didn't exist - remove it
65+
if ( overwritten === undefined ) {
66+
jQuery( window ).removeProp( callbackName );
7067

71-
// Otherwise restore preexisting value
72-
} else {
73-
window[ callbackName ] = overwritten;
74-
}
68+
// Otherwise restore preexisting value
69+
} else {
70+
window[ callbackName ] = overwritten;
71+
}
7572

76-
// Save back as free
77-
if ( s[ callbackName ] ) {
73+
// Save back as free
74+
if ( s[ callbackName ] ) {
7875

79-
// Make sure that re-using the options doesn't screw things around
80-
s.jsonpCallback = originalSettings.jsonpCallback;
76+
// Make sure that re-using the options doesn't screw things around
77+
s.jsonpCallback = originalSettings.jsonpCallback;
8178

82-
// Save the callback name for future use
83-
oldCallbacks.push( callbackName );
84-
}
79+
// Save the callback name for future use
80+
oldCallbacks.push( callbackName );
81+
}
8582

86-
// Call if it was a function and we have a response
87-
if ( responseContainer && typeof overwritten === "function" ) {
88-
overwritten( responseContainer[ 0 ] );
89-
}
83+
// Call if it was a function and we have a response
84+
if ( responseContainer && typeof overwritten === "function" ) {
85+
overwritten( responseContainer[ 0 ] );
86+
}
9087

91-
responseContainer = overwritten = undefined;
92-
} );
88+
responseContainer = overwritten = undefined;
89+
} );
9390

94-
// Delegate to script
95-
return "script";
96-
}
91+
// Delegate to script
92+
return "script";
9793
} );

‎test/data/json.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{bad: toTheBone}

‎test/data/mock.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ protected function json( $req ) {
7070
header( 'Content-type: application/json' );
7171
}
7272

73+
if ( isset( $req->query['cors'] ) ) {
74+
header( 'Access-Control-Allow-Origin: *' );
75+
}
76+
7377
if ( isset( $req->query['array'] ) ) {
7478
echo '[ {"name": "John", "age": 21}, {"name": "Peter", "age": 25 } ]';
7579
} else {

‎test/middleware-mockserver.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ var mocks = {
8181
if ( req.query.header ) {
8282
resp.writeHead( 200, { "content-type": "application/json" } );
8383
}
84+
if ( req.query.cors ) {
85+
resp.writeHead( 200, { "access-control-allow-origin": "*" } );
86+
}
8487
if ( req.query.array ) {
8588
resp.end( JSON.stringify(
8689
[ { name: "John", age: 21 }, { name: "Peter", age: 25 } ]

‎test/unit/ajax.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,45 @@ QUnit.module( "ajax", {
12391239
];
12401240
} );
12411241

1242+
ajaxTest( "jQuery.ajax() - no JSONP auto-promotion" + label, 3, function( assert ) {
1243+
return [
1244+
{
1245+
url: baseURL + "mock.php?action=jsonp",
1246+
dataType: "json",
1247+
crossDomain: crossDomain,
1248+
success: function() {
1249+
assert.ok( false, "JSON parsing should have failed (no callback)" );
1250+
},
1251+
fail: function() {
1252+
assert.ok( true, "JSON parsing failed, JSONP not used (no callback)" );
1253+
}
1254+
},
1255+
{
1256+
url: baseURL + "mock.php?action=jsonp",
1257+
dataType: "json",
1258+
crossDomain: crossDomain,
1259+
data: "callback=?",
1260+
success: function() {
1261+
assert.ok( false, "JSON parsing should have failed (with callback)" );
1262+
},
1263+
fail: function() {
1264+
assert.ok( true, "JSON parsing failed, JSONP not used (with callback)" );
1265+
}
1266+
},
1267+
{
1268+
url: baseURL + "mock.php?action=jsonp&callback=?",
1269+
dataType: "json",
1270+
crossDomain: crossDomain,
1271+
success: function() {
1272+
assert.ok( false, "JSON parsing should have failed (with callback)" );
1273+
},
1274+
fail: function() {
1275+
assert.ok( true, "JSON parsing failed, JSONP not used (with callback)" );
1276+
}
1277+
}
1278+
];
1279+
} );
1280+
12421281
} );
12431282

12441283
ajaxTest( "jQuery.ajax() - script, Remote", 2, function( assert ) {

0 commit comments

Comments
 (0)