Ajax: Fill in & warn against automatic JSON-to-JSONP promotion

So far, the patch was only warning about the automatic promotion, but it wasn't
filling the behavior back to jQuery 4+. This has been fixed.

Closes gh-531

eslint.config.js

@@ -130,6 +130,7 @@ export default [
 				QUnit: false,
 				url: true,
 				compareVersions: true,
+				jQueryVersionSince: false,
 				expectWarning: true,
 				expectNoWarning: true,
 				startIframeTest: true,

src/jquery/ajax.js

@@ -5,7 +5,11 @@ import { migrateWarn, migratePatchAndWarnFunc, migratePatchFunc } from "../main.
 if ( jQuery.ajax ) {
 
 var oldAjax = jQuery.ajax,
-	rjsonp = /(=)\?(?=&|$)|\?\?/;
+	oldCallbacks = [],
+	guid = "migrate-" + Date.now(),
+	origJsonpCallback = jQuery.ajaxSettings.jsonpCallback,
+	rjsonp = /(=)\?(?=&|$)|\?\?/,
+	rquery = /\?/;
 
 migratePatchFunc( jQuery, "ajax", function() {
 	var jQXHR = oldAjax.apply( this, arguments );
@@ -23,16 +27,120 @@ migratePatchFunc( jQuery, "ajax", function() {
 	return jQXHR;
 }, "jqXHR-methods" );
 
-// Only trigger the logic in jQuery <4 as the JSON-to-JSONP auto-promotion
-// behavior is gone in jQuery 4.0 and as it has security implications, we don't
-// want to restore the legacy behavior.
-if ( !jQueryVersionSince( "4.0.0" ) ) {
+jQuery.ajaxSetup( {
+	jsonpCallback: function() {
 
-	// Register this prefilter before the jQuery one. Otherwise, a promoted
-	// request is transformed into one with the script dataType and we can't
-	// catch it anymore.
+		// Source is virtually the same as in Core, but we need to duplicate
+		// to maintain a proper `oldCallbacks` reference.
+		if ( jQuery.migrateIsPatchEnabled( "jsonp-promotion" ) ) {
+			var callback = oldCallbacks.pop() || ( jQuery.expando + "_" + ( guid++ ) );
+			this[ callback ] = true;
+			return callback;
+		} else {
+			return origJsonpCallback.apply( this, arguments );
+		}
+	}
+} );
+
+// Register this prefilter before the jQuery one. Otherwise, a promoted
+// request is transformed into one with the script dataType, and we can't
+// catch it anymore.
+if ( jQueryVersionSince( "4.0.0" ) ) {
+
+	// Code mostly from:
+	// https://github.com/jquery/jquery/blob/fa0058af426c4e482059214c29c29f004254d9a1/src/ajax/jsonp.js#L20-L97
+	jQuery.ajaxPrefilter( "+json", function( s, originalSettings, jqXHR ) {
+
+		if ( !jQuery.migrateIsPatchEnabled( "jsonp-promotion" ) ) {
+			return;
+		}
+
+		var callbackName, overwritten, responseContainer,
+			jsonProp = s.jsonp !== false && ( rjsonp.test( s.url ) ?
+				"url" :
+				typeof s.data === "string" &&
+					( s.contentType || "" )
+						.indexOf( "application/x-www-form-urlencoded" ) === 0 &&
+					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" ) {
+			migrateWarn( "jsonp-promotion", "JSON-to-JSONP auto-promotion is deprecated" );
+
+			// 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;
+			}
+
+			// 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";
+
+			// Install callback
+			overwritten = window[ callbackName ];
+			window[ callbackName ] = function() {
+				responseContainer = arguments;
+			};
+
+			// Clean-up function (fires after converters)
+			jqXHR.always( function() {
+
+				// If previous value didn't exist - remove it
+				if ( overwritten === undefined ) {
+					jQuery( window ).removeProp( callbackName );
+
+					// Otherwise restore preexisting value
+				} else {
+					window[ callbackName ] = overwritten;
+				}
+
+				// Save back as free
+				if ( s[ callbackName ] ) {
+
+					// 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 );
+				}
+
+				// Call if it was a function and we have a response
+				if ( responseContainer && typeof overwritten === "function" ) {
+					overwritten( responseContainer[ 0 ] );
+				}
+
+				responseContainer = overwritten = undefined;
+			} );
+
+			// Delegate to script
+			return "script";
+		}
+	} );
+} else {
+
+	// jQuery <4 already contains this prefixer; don't duplicate the whole logic,
+	// but only enough to know when to warn.
 	jQuery.ajaxPrefilter( "+json", function( s ) {
 
+		if ( !jQuery.migrateIsPatchEnabled( "jsonp-promotion" ) ) {
+			return;
+		}
+
 		// Warn if JSON-to-JSONP auto-promotion happens.
 		if ( s.jsonp !== false && ( rjsonp.test( s.url ) ||
 				typeof s.data === "string" &&
@@ -45,4 +153,12 @@ if ( !jQueryVersionSince( "4.0.0" ) ) {
 	} );
 }
 
+
+// Don't trigger the above logic in jQuery >=4 by default as the JSON-to-JSONP
+// auto-promotion behavior is gone in jQuery 4.0 and as it has security implications,
+// we don't want to restore the legacy behavior by default.
+if ( jQueryVersionSince( "4.0.0" ) ) {
+	jQuery.migrateDisablePatches( "jsonp-promotion" );
+}
+
 }

test/data/ajax-jsonp-callback-name.html

@@ -0,0 +1,87 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta charset="utf-8">
+	<title>jQuery Migrate test for re-using JSONP callback name with `dataType: "json"`</title>
+
+	<!-- Load a jQuery and jquery-migrate plugin file based on URL -->
+	<script src="testinit.js"></script>
+	<script>
+		TestManager.loadProject( "jquery", "git" );
+	</script>
+	<script src="iframeTest.js"></script>
+	<script>
+		jQuery.noConflict();
+		TestManager.loadProject( "jquery-migrate", "dev", true );
+	</script>
+	<script>
+		var thisCallbackInWindow1, thisCallbackInWindow2,
+			previousCallback, previousJsonpCallback,
+			nextJsonpCallback, error,
+			requestSucceeded = true;
+
+		jQuery.migrateEnablePatches( "jsonp-promotion" )
+
+		jQuery.ajaxTransport( "script", function( options ) {
+			if ( options.url.indexOf( 'fakeScript' ) === -1 ) {
+				return;
+			}
+			return {
+				abort: function () {},
+				send: function ( headers, completeCallback ) {
+					var script = options.jsonpCallback + "( { answer: 42 } )";
+					setTimeout( function() {
+						completeCallback( 200, 'OK', { text: script }, [] );
+					} );
+				}
+			};
+		} );
+
+		jQuery.ajax( {
+			url: "fakeScript1.js?callback=?",
+			dataType: "json",
+			beforeSend: function( jqXHR, s ) {
+				s.callback = s.jsonpCallback;
+
+				thisCallbackInWindow1 = this.callback in window;
+			}
+		} ).then( function() {
+			var previous = this;
+
+			previousJsonpCallback = previous.jsonpCallback;
+			thisCallbackInWindow2 = this.callback in window;
+
+			return jQuery.ajax( {
+				url: "fakeScript2.js?callback=?",
+				dataType: "json",
+				beforeSend: function() {
+					previousCallback = previous.callback;
+					nextJsonpCallback = this.jsonpCallback;
+
+					// Test cleanup
+					delete window.customJsonpCallback;
+				}
+			} );
+		} ).catch( function( _jqXHR, _textStatus, err ) {
+
+			if ( !( err instanceof Error ) && _jqXHR instanceof Error ) {
+				err = _jqXHR;
+			}
+
+			// Test cleanup in case of an error
+			delete window.customJsonpCallback;
+			requestSucceeded = false;
+			error = err;
+		} ).then( function() {
+			startIframeTest(
+				thisCallbackInWindow1, thisCallbackInWindow2,
+				previousJsonpCallback, previousCallback,
+				nextJsonpCallback, requestSucceeded, error
+			);
+		} );
+	</script>
+</head>
+<body>
+<p>jQuery Migrate test for re-using JSONP callback name</p>
+</body>
+</html>

test/data/jsonpScript.js

@@ -0,0 +1,5 @@
+/* global customJsonpCallback */
+
+"use strict";
+
+customJsonpCallback( { answer: 42 } );

test/data/testinit.js

@@ -234,6 +234,9 @@
 
 				// Re-disable patches disabled by default
 				jQuery.migrateDisablePatches( "self-closed-tags" );
+				if ( jQueryVersionSince( "4.0.0" ) ) {
+					jQuery.migrateDisablePatches( "jsonp-promotion" );
+				}
 			}
 		} );
 	}

test/index.html

@@ -14,6 +14,9 @@
 	<!-- A promise polyfill -->
 	<script src="../external/npo/npo.js"></script>
 
+	<!-- Version comparisons -->
+	<script src="data/compareVersions.js"></script>
+
 	<!-- Load a jQuery and jquery-migrate plugin file based on URL -->
 	<script src="data/testinit.js"></script>
 	<script>
@@ -25,9 +28,6 @@
 		TestManager.loadProject( "jquery-migrate", "min", true );
 	</script>
 
-	<!-- Version comparisons -->
-	<script src="data/compareVersions.js"></script>
-
 	<!-- Unit test files -->
 	<script>
 		TestManager.loadTests();