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

diff --git a/src/ajax/jsonp.js b/src/ajax/jsonp.js
index 1d612c1a976c4a846699de011fa727226b407daf..e115fdafcdcf6ee886c1daa646ca7f9709deded8 100644 (file)
--- 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 5b56d02c7e0740caf5c4535e1c1a6e1c2cd5ecb5..a34e8f5ace9f68b6874dd5813af79fceb9103792 100644 (file)
--- 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 f6196d2307e4d2b13d79bf34752fe97f03f3706e..e3b0bd163032d2b55e0cf305257b7d2e0ce1b6d4 100644 (file)
--- 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 e13c2713d2a7111c3b41711acf22462be41d5c5b..cd7822b73a89a0b25c11a5f48cf794a6e75f4669 100644 (file)
--- 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 ) {