Skip to content

Commit e7b3bc4

Browse files
mgolmgol
authored
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-1799 Fixes gh-3376 Closes gh-4754
1 parent fa0058a commit e7b3bc4

File tree

4 files changed

+170
-52
lines changed

4 files changed

+170
-52
lines changed

‎src/ajax/jsonp.js

+48-52
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/mock.php

+4
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

+3
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

+115
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,121 @@ QUnit.module( "ajax", {
12391239
];
12401240
} );
12411241

1242+
ajaxTest( "jQuery.ajax() - no JSONP auto-promotion" + label, 4, 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&callback=?",
1257+
dataType: "json",
1258+
crossDomain: crossDomain,
1259+
success: function() {
1260+
assert.ok( false, "JSON parsing should have failed (ULR callback)" );
1261+
},
1262+
fail: function() {
1263+
assert.ok( true, "JSON parsing failed, JSONP not used (URL callback)" );
1264+
}
1265+
},
1266+
{
1267+
url: baseURL + "mock.php?action=jsonp",
1268+
dataType: "json",
1269+
crossDomain: crossDomain,
1270+
data: "callback=?",
1271+
success: function() {
1272+
assert.ok( false, "JSON parsing should have failed (data callback=?)" );
1273+
},
1274+
fail: function() {
1275+
assert.ok( true, "JSON parsing failed, JSONP not used (data callback=?)" );
1276+
}
1277+
},
1278+
{
1279+
url: baseURL + "mock.php?action=jsonp",
1280+
dataType: "json",
1281+
crossDomain: crossDomain,
1282+
data: "callback=??",
1283+
success: function() {
1284+
assert.ok( false, "JSON parsing should have failed (data callback=??)" );
1285+
},
1286+
fail: function() {
1287+
assert.ok( true, "JSON parsing failed, JSONP not used (data callback=??)" );
1288+
}
1289+
}
1290+
];
1291+
} );
1292+
1293+
ajaxTest( "jQuery.ajax() - JSON - no ? replacement" + label, 9, function( assert ) {
1294+
return [
1295+
{
1296+
url: baseURL + "mock.php?action=json&callback=?",
1297+
dataType: "json",
1298+
crossDomain: crossDomain,
1299+
beforeSend: function( _jqXhr, settings ) {
1300+
var queryString = settings.url.replace( /^[^?]*\?/, "" );
1301+
assert.ok(
1302+
queryString.indexOf( "jQuery" ) === -1,
1303+
"jQuery callback not inserted into the URL (URL callback)"
1304+
);
1305+
assert.ok(
1306+
queryString.indexOf( "callback=?" ) > -1,
1307+
"\"callback=?\" present in the URL unchanged (URL callback)"
1308+
);
1309+
},
1310+
success: function( data ) {
1311+
assert.ok( data.data, "JSON results returned (URL callback)" );
1312+
}
1313+
},
1314+
{
1315+
url: baseURL + "mock.php?action=json",
1316+
dataType: "json",
1317+
crossDomain: crossDomain,
1318+
data: "callback=?",
1319+
beforeSend: function( _jqXhr, settings ) {
1320+
var queryString = settings.url.replace( /^[^?]*\?/, "" );
1321+
assert.ok(
1322+
queryString.indexOf( "jQuery" ) === -1,
1323+
"jQuery callback not inserted into the URL (data callback=?)"
1324+
);
1325+
assert.ok(
1326+
queryString.indexOf( "callback=?" ) > -1,
1327+
"\"callback=?\" present in the URL unchanged (data callback=?)"
1328+
);
1329+
},
1330+
success: function( data ) {
1331+
assert.ok( data.data, "JSON results returned (data callback=?)" );
1332+
}
1333+
},
1334+
{
1335+
url: baseURL + "mock.php?action=json",
1336+
dataType: "json",
1337+
crossDomain: crossDomain,
1338+
data: "callback=??",
1339+
beforeSend: function( _jqXhr, settings ) {
1340+
var queryString = settings.url.replace( /^[^?]*\?/, "" );
1341+
assert.ok(
1342+
queryString.indexOf( "jQuery" ) === -1,
1343+
"jQuery callback not inserted into the URL (data callback=??)"
1344+
);
1345+
assert.ok(
1346+
queryString.indexOf( "callback=??" ) > -1,
1347+
"\"callback=?\" present in the URL unchanged (data callback=??)"
1348+
);
1349+
},
1350+
success: function( data ) {
1351+
assert.ok( data.data, "JSON results returned (data callback=??)" );
1352+
}
1353+
}
1354+
];
1355+
} );
1356+
12421357
} );
12431358

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

0 commit comments

Comments
 (0)