From e7b3bc488d01d584262e12a7c5c25f935d0d034b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Mon, 27 Jul 2020 19:15:57 +0200 Subject: [PATCH] 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 --- src/ajax/jsonp.js | 100 ++++++++++++++--------------- test/data/mock.php | 4 ++ test/middleware-mockserver.js | 3 + test/unit/ajax.js | 115 ++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 52 deletions(-) diff --git a/src/ajax/jsonp.js b/src/ajax/jsonp.js index 1d612c1a9..e115fdafc 100644 --- a/src/ajax/jsonp.js +++ b/src/ajax/jsonp.js @@ -18,7 +18,7 @@ jQuery.ajaxSetup( { } ); // Detect, normalize options and install callbacks for jsonp requests -jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, jqXHR ) { +jQuery.ajaxPrefilter( "jsonp", function( s, originalSettings, jqXHR ) { var callbackName, overwritten, responseContainer, jsonProp = s.jsonp !== false && ( rjsonp.test( s.url ) ? @@ -29,69 +29,65 @@ jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, jqXHR ) { rjsonp.test( s.data ) && "data" ); - // Handle iff the expected data type is "jsonp" or we have a parameter to set - if ( jsonProp || s.dataTypes[ 0 ] === "jsonp" ) { + // Get callback name, remembering preexisting value associated with it + callbackName = s.jsonpCallback = typeof s.jsonpCallback === "function" ? + s.jsonpCallback() : + s.jsonpCallback; - // Get callback name, remembering preexisting value associated with it - callbackName = s.jsonpCallback = typeof s.jsonpCallback === "function" ? - s.jsonpCallback() : - s.jsonpCallback; + // Insert callback into url or form data + if ( jsonProp ) { + s[ jsonProp ] = s[ jsonProp ].replace( rjsonp, "$1" + callbackName ); + } else if ( s.jsonp !== false ) { + s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.jsonp + "=" + callbackName; + } - // Insert callback into url or form data - if ( jsonProp ) { - s[ jsonProp ] = s[ jsonProp ].replace( rjsonp, "$1" + callbackName ); - } else if ( s.jsonp !== false ) { - s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.jsonp + "=" + callbackName; + // Use data converter to retrieve json after script execution + s.converters[ "script json" ] = function() { + if ( !responseContainer ) { + jQuery.error( callbackName + " was not called" ); } + return responseContainer[ 0 ]; + }; - // Use data converter to retrieve json after script execution - s.converters[ "script json" ] = function() { - if ( !responseContainer ) { - jQuery.error( callbackName + " was not called" ); - } - return responseContainer[ 0 ]; - }; - - // Force json dataType - s.dataTypes[ 0 ] = "json"; + // Force json dataType + s.dataTypes[ 0 ] = "json"; - // Install callback - overwritten = window[ callbackName ]; - window[ callbackName ] = function() { - responseContainer = arguments; - }; + // Install callback + overwritten = window[ callbackName ]; + window[ callbackName ] = function() { + responseContainer = arguments; + }; - // Clean-up function (fires after converters) - jqXHR.always( function() { + // Clean-up function (fires after converters) + jqXHR.always( function() { - // If previous value didn't exist - remove it - if ( overwritten === undefined ) { - jQuery( window ).removeProp( callbackName ); + // If previous value didn't exist - remove it + if ( overwritten === undefined ) { + jQuery( window ).removeProp( callbackName ); - // Otherwise restore preexisting value - } else { - window[ callbackName ] = overwritten; - } + // Otherwise restore preexisting value + } else { + window[ callbackName ] = overwritten; + } - // Save back as free - if ( s[ callbackName ] ) { + // Save back as free + if ( s[ callbackName ] ) { - // Make sure that re-using the options doesn't screw things around - s.jsonpCallback = originalSettings.jsonpCallback; + // Make sure that re-using the options doesn't screw things around + s.jsonpCallback = originalSettings.jsonpCallback; - // Save the callback name for future use - oldCallbacks.push( callbackName ); - } + // Save the callback name for future use + oldCallbacks.push( callbackName ); + } - // Call if it was a function and we have a response - if ( responseContainer && typeof overwritten === "function" ) { - overwritten( responseContainer[ 0 ] ); - } + // Call if it was a function and we have a response + if ( responseContainer && typeof overwritten === "function" ) { + overwritten( responseContainer[ 0 ] ); + } - responseContainer = overwritten = undefined; - } ); + responseContainer = overwritten = undefined; + } ); - // Delegate to script - return "script"; - } + // Delegate to script + return "script"; } ); diff --git a/test/data/mock.php b/test/data/mock.php index 5b56d02c7..a34e8f5ac 100644 --- a/test/data/mock.php +++ b/test/data/mock.php @@ -70,6 +70,10 @@ QUnit.assert.ok( true, "mock executed");'; header( 'Content-type: application/json' ); } + if ( isset( $req->query['cors'] ) ) { + header( 'Access-Control-Allow-Origin: *' ); + } + if ( isset( $req->query['array'] ) ) { echo '[ {"name": "John", "age": 21}, {"name": "Peter", "age": 25 } ]'; } else { diff --git a/test/middleware-mockserver.js b/test/middleware-mockserver.js index f6196d230..e3b0bd163 100644 --- a/test/middleware-mockserver.js +++ b/test/middleware-mockserver.js @@ -81,6 +81,9 @@ var mocks = { if ( req.query.header ) { resp.writeHead( 200, { "content-type": "application/json" } ); } + if ( req.query.cors ) { + resp.writeHead( 200, { "access-control-allow-origin": "*" } ); + } if ( req.query.array ) { resp.end( JSON.stringify( [ { name: "John", age: 21 }, { name: "Peter", age: 25 } ] diff --git a/test/unit/ajax.js b/test/unit/ajax.js index e13c2713d..cd7822b73 100644 --- a/test/unit/ajax.js +++ b/test/unit/ajax.js @@ -1239,6 +1239,121 @@ QUnit.module( "ajax", { ]; } ); + ajaxTest( "jQuery.ajax() - no JSONP auto-promotion" + label, 4, function( assert ) { + return [ + { + url: baseURL + "mock.php?action=jsonp", + dataType: "json", + crossDomain: crossDomain, + success: function() { + assert.ok( false, "JSON parsing should have failed (no callback)" ); + }, + fail: function() { + assert.ok( true, "JSON parsing failed, JSONP not used (no callback)" ); + } + }, + { + url: baseURL + "mock.php?action=jsonp&callback=?", + dataType: "json", + crossDomain: crossDomain, + success: function() { + assert.ok( false, "JSON parsing should have failed (ULR callback)" ); + }, + fail: function() { + assert.ok( true, "JSON parsing failed, JSONP not used (URL callback)" ); + } + }, + { + url: baseURL + "mock.php?action=jsonp", + dataType: "json", + crossDomain: crossDomain, + data: "callback=?", + success: function() { + assert.ok( false, "JSON parsing should have failed (data callback=?)" ); + }, + fail: function() { + assert.ok( true, "JSON parsing failed, JSONP not used (data callback=?)" ); + } + }, + { + url: baseURL + "mock.php?action=jsonp", + dataType: "json", + crossDomain: crossDomain, + data: "callback=??", + success: function() { + assert.ok( false, "JSON parsing should have failed (data callback=??)" ); + }, + fail: function() { + assert.ok( true, "JSON parsing failed, JSONP not used (data callback=??)" ); + } + } + ]; + } ); + + ajaxTest( "jQuery.ajax() - JSON - no ? replacement" + label, 9, function( assert ) { + return [ + { + url: baseURL + "mock.php?action=json&callback=?", + dataType: "json", + crossDomain: crossDomain, + beforeSend: function( _jqXhr, settings ) { + var queryString = settings.url.replace( /^[^?]*\?/, "" ); + assert.ok( + queryString.indexOf( "jQuery" ) === -1, + "jQuery callback not inserted into the URL (URL callback)" + ); + assert.ok( + queryString.indexOf( "callback=?" ) > -1, + "\"callback=?\" present in the URL unchanged (URL callback)" + ); + }, + success: function( data ) { + assert.ok( data.data, "JSON results returned (URL callback)" ); + } + }, + { + url: baseURL + "mock.php?action=json", + dataType: "json", + crossDomain: crossDomain, + data: "callback=?", + beforeSend: function( _jqXhr, settings ) { + var queryString = settings.url.replace( /^[^?]*\?/, "" ); + assert.ok( + queryString.indexOf( "jQuery" ) === -1, + "jQuery callback not inserted into the URL (data callback=?)" + ); + assert.ok( + queryString.indexOf( "callback=?" ) > -1, + "\"callback=?\" present in the URL unchanged (data callback=?)" + ); + }, + success: function( data ) { + assert.ok( data.data, "JSON results returned (data callback=?)" ); + } + }, + { + url: baseURL + "mock.php?action=json", + dataType: "json", + crossDomain: crossDomain, + data: "callback=??", + beforeSend: function( _jqXhr, settings ) { + var queryString = settings.url.replace( /^[^?]*\?/, "" ); + assert.ok( + queryString.indexOf( "jQuery" ) === -1, + "jQuery callback not inserted into the URL (data callback=??)" + ); + assert.ok( + queryString.indexOf( "callback=??" ) > -1, + "\"callback=?\" present in the URL unchanged (data callback=??)" + ); + }, + success: function( data ) { + assert.ok( data.data, "JSON results returned (data callback=??)" ); + } + } + ]; + } ); + } ); ajaxTest( "jQuery.ajax() - script, Remote", 2, function( assert ) { -- 2.39.5