Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
  • Loading branch information
mgol committed Sep 16, 2024
commit 560ce28965e057cc302f438aba19f284b28046c6
1 change: 1 addition & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export default [
QUnit: false,
url: true,
compareVersions: true,
jQueryVersionSince: false,
expectWarning: true,
expectNoWarning: true,
startIframeTest: true,
Expand Down
132 changes: 124 additions & 8 deletions src/jquery/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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" &&
Expand All @@ -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" );
}

}
5 changes: 5 additions & 0 deletions test/data/jsonpScript.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/* global customJsonpCallback */

"use strict";

customJsonpCallback( { answer: 42 } );
3 changes: 3 additions & 0 deletions test/data/testinit.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@

// Re-disable patches disabled by default
jQuery.migrateDisablePatches( "self-closed-tags" );
if ( jQueryVersionSince( "4.0.0" ) ) {
jQuery.migrateDisablePatches( "jsonp-promotion" );
}
}
} );
}
Expand Down
6 changes: 3 additions & 3 deletions test/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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();
Expand Down
Loading