Skip to content

Commit 87d27c7

Browse files
committed
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.
1 parent 95d05ce commit 87d27c7

File tree

6 files changed

+235
-85
lines changed

6 files changed

+235
-85
lines changed

eslint.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export default [
126126
QUnit: false,
127127
url: true,
128128
compareVersions: true,
129+
jQueryVersionSince: false,
129130
expectWarning: true,
130131
expectNoWarning: true,
131132
startIframeTest: true,

src/jquery/ajax.js

Lines changed: 107 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import { migrateWarn, migratePatchAndWarnFunc, migratePatchFunc } from "../main.
55
if ( jQuery.ajax ) {
66

77
var oldAjax = jQuery.ajax,
8-
rjsonp = /(=)\?(?=&|$)|\?\?/;
8+
oldCallbacks = [],
9+
rjsonp = /(=)\?(?=&|$)|\?\?/,
10+
rquery = /\?/;
911

1012
migratePatchFunc( jQuery, "ajax", function() {
1113
var jQXHR = oldAjax.apply( this, arguments );
@@ -23,16 +25,105 @@ migratePatchFunc( jQuery, "ajax", function() {
2325
return jQXHR;
2426
}, "jqXHR-methods" );
2527

26-
// Only trigger the logic in jQuery <4 as the JSON-to-JSONP auto-promotion
27-
// behavior is gone in jQuery 4.0 and as it has security implications, we don't
28-
// want to restore the legacy behavior.
29-
if ( !jQueryVersionSince( "4.0.0" ) ) {
28+
// Register this prefilter before the jQuery one. Otherwise, a promoted
29+
// request is transformed into one with the script dataType, and we can't
30+
// catch it anymore.
31+
if ( jQueryVersionSince( "4.0.0" ) ) {
3032

31-
// Register this prefilter before the jQuery one. Otherwise, a promoted
32-
// request is transformed into one with the script dataType and we can't
33-
// catch it anymore.
33+
// Code mostly from:
34+
// https://github.com/jquery/jquery/blob/fa0058af426c4e482059214c29c29f004254d9a1/src/ajax/jsonp.js#L20-L97
35+
jQuery.ajaxPrefilter( "+json", function( s, originalSettings, jqXHR ) {
36+
37+
if ( !jQuery.migrateIsPatchEnabled( "jsonp-promotion" ) ) {
38+
return;
39+
}
40+
41+
var callbackName, overwritten, responseContainer,
42+
jsonProp = s.jsonp !== false && ( rjsonp.test( s.url ) ?
43+
"url" :
44+
typeof s.data === "string" &&
45+
( s.contentType || "" )
46+
.indexOf( "application/x-www-form-urlencoded" ) === 0 &&
47+
rjsonp.test( s.data ) && "data"
48+
);
49+
50+
// Handle iff the expected data type is "jsonp" or we have a parameter to set
51+
if ( jsonProp || s.dataTypes[ 0 ] === "jsonp" ) {
52+
migrateWarn( "jsonp-promotion", "JSON-to-JSONP auto-promotion is deprecated" );
53+
54+
// Get callback name, remembering preexisting value associated with it
55+
callbackName = s.jsonpCallback = typeof s.jsonpCallback === "function" ?
56+
s.jsonpCallback() :
57+
s.jsonpCallback;
58+
59+
// Insert callback into url or form data
60+
if ( jsonProp ) {
61+
s[ jsonProp ] = s[ jsonProp ].replace( rjsonp, "$1" + callbackName );
62+
} else if ( s.jsonp !== false ) {
63+
s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.jsonp + "=" + callbackName;
64+
}
65+
66+
// Use data converter to retrieve json after script execution
67+
s.converters[ "script json" ] = function() {
68+
if ( !responseContainer ) {
69+
jQuery.error( callbackName + " was not called" );
70+
}
71+
return responseContainer[ 0 ];
72+
};
73+
74+
// Force json dataType
75+
s.dataTypes[ 0 ] = "json";
76+
77+
// Install callback
78+
overwritten = window[ callbackName ];
79+
window[ callbackName ] = function() {
80+
responseContainer = arguments;
81+
};
82+
83+
// Clean-up function (fires after converters)
84+
jqXHR.always( function() {
85+
86+
// If previous value didn't exist - remove it
87+
if ( overwritten === undefined ) {
88+
jQuery( window ).removeProp( callbackName );
89+
90+
// Otherwise restore preexisting value
91+
} else {
92+
window[ callbackName ] = overwritten;
93+
}
94+
95+
// Save back as free
96+
if ( s[ callbackName ] ) {
97+
98+
// Make sure that re-using the options doesn't screw things around
99+
s.jsonpCallback = originalSettings.jsonpCallback;
100+
101+
// Save the callback name for future use
102+
oldCallbacks.push( callbackName );
103+
}
104+
105+
// Call if it was a function and we have a response
106+
if ( responseContainer && typeof overwritten === "function" ) {
107+
overwritten( responseContainer[ 0 ] );
108+
}
109+
110+
responseContainer = overwritten = undefined;
111+
} );
112+
113+
// Delegate to script
114+
return "script";
115+
}
116+
} );
117+
} else {
118+
119+
// jQuery <4 already contains this prefixer; don't duplicate the whole logic,
120+
// but only enough to know when to warn.
34121
jQuery.ajaxPrefilter( "+json", function( s ) {
35122

123+
if ( !jQuery.migrateIsPatchEnabled( "jsonp-promotion" ) ) {
124+
return;
125+
}
126+
36127
// Warn if JSON-to-JSONP auto-promotion happens.
37128
if ( s.jsonp !== false && ( rjsonp.test( s.url ) ||
38129
typeof s.data === "string" &&
@@ -45,4 +136,12 @@ if ( !jQueryVersionSince( "4.0.0" ) ) {
45136
} );
46137
}
47138

139+
140+
// Don't trigger the above logic in jQuery >=4 by default as the JSON-to-JSONP
141+
// auto-promotion behavior is gone in jQuery 4.0 and as it has security implications,
142+
// we don't want to restore the legacy behavior by default.
143+
if ( jQueryVersionSince( "4.0.0" ) ) {
144+
jQuery.migrateDisablePatches( "jsonp-promotion" );
145+
}
146+
48147
}

test/data/jsonpScript.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/* global customJsonpCallback */
2+
3+
"use strict";
4+
5+
customJsonpCallback( { answer: 42 } );

test/data/testinit.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,9 @@
233233

234234
// Re-disable patches disabled by default
235235
jQuery.migrateDisablePatches( "self-closed-tags" );
236+
if ( jQueryVersionSince( "4.0.0" ) ) {
237+
jQuery.migrateDisablePatches( "jsonp-promotion" );
238+
}
236239
}
237240
} );
238241
}

test/index.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
<!-- A promise polyfill -->
1515
<script src="../external/npo/npo.js"></script>
1616

17+
<!-- Version comparisons -->
18+
<script src="data/compareVersions.js"></script>
19+
1720
<!-- Load a jQuery and jquery-migrate plugin file based on URL -->
1821
<script src="data/testinit.js"></script>
1922
<script>
@@ -25,9 +28,6 @@
2528
TestManager.loadProject( "jquery-migrate", "min", true );
2629
</script>
2730

28-
<!-- Version comparisons -->
29-
<script src="data/compareVersions.js"></script>
30-
3131
<!-- Unit test files -->
3232
<script>
3333
TestManager.loadTests();

test/unit/jquery/ajax.js

Lines changed: 116 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -27,95 +27,137 @@ QUnit.test( "jQuery.ajax() deprecations on jqXHR", function( assert ) {
2727
} );
2828

2929
[ " - Same Domain", " - Cross Domain" ].forEach( function( label, crossDomain ) {
30+
function runTests( options ) {
31+
var forceEnablePatch = ( options || {} ).forceEnablePatch || false;
3032

31-
// The JSON-to-JSONP auto-promotion behavior is gone in jQuery 4.0 and as
32-
// it has security implications, we don't want to restore the legacy behavior.
33-
QUnit[ jQueryVersionSince( "4.0.0" ) ? "skip" : "test" ](
34-
"jQuery.ajax() JSON-to-JSONP auto-promotion" + label, function( assert ) {
33+
QUnit.test( "jQuery.ajax() JSON-to-JSONP auto-promotion" + label + (
34+
forceEnablePatch ? ", patch force-enabled" : ""
35+
), function( assert ) {
3536

36-
assert.expect( 5 );
37+
assert.expect( 10 );
3738

38-
var done = assert.async(),
39-
tests = [
40-
function() {
41-
return expectNoWarning( assert, "dataType: \"json\"",
42-
function() {
39+
if ( forceEnablePatch ) {
40+
jQuery.migrateEnablePatches( "jsonp-promotion" );
41+
}
42+
43+
var done = assert.async(),
44+
patchEnabled = forceEnablePatch || !jQueryVersionSince( "4.0.0" ),
45+
tests = [
46+
function() {
47+
var testName = "dataType: \"json\"";
48+
return expectNoWarning( assert, testName, function() {
4349
return jQuery.ajax( {
4450
url: url( "null.json" ),
51+
context: { testName: testName },
4552
crossDomain: crossDomain,
46-
dataType: "json"
47-
} ).catch( jQuery.noop );
48-
}
49-
);
50-
},
51-
52-
function() {
53-
return expectWarning( assert, "dataType: \"json\", URL callback", 1,
54-
function() {
53+
dataType: "json",
54+
jsonpCallback: "customJsonpCallback"
55+
} ).then( function() {
56+
assert.ok( true, this.testName + " (success)" );
57+
} ).catch( function() {
58+
assert.ok( false, this.testName + " (failure)" );
59+
} );
60+
} );
61+
},
62+
63+
function() {
64+
var testName = "dataType: \"json\", URL callback";
65+
return expectWarning( assert, testName, patchEnabled ? 1 : 0, function() {
5566
return jQuery.ajax( {
56-
url: url( "null.json?callback=?" ),
67+
url: url( "jsonpScript.js?callback=?" ),
68+
context: { testName: testName },
5769
crossDomain: crossDomain,
58-
dataType: "json"
59-
} ).catch( jQuery.noop );
60-
}
61-
);
62-
},
63-
64-
function() {
65-
return expectWarning( assert, "dataType: \"json\", data callback", 1,
66-
function() {
70+
dataType: "json",
71+
jsonpCallback: "customJsonpCallback"
72+
} ).then( function() {
73+
assert.ok( patchEnabled, this.testName + " (success)" );
74+
} ).catch( function() {
75+
assert.ok( !patchEnabled, this.testName + " (failure)" );
76+
} );
77+
} );
78+
},
79+
80+
function() {
81+
var testName = "dataType: \"json\", data callback";
82+
return expectWarning( assert, testName, patchEnabled ? 1 : 0, function() {
6783
return jQuery.ajax( {
68-
url: url( "null.json" ),
84+
url: url( "jsonpScript.js" ),
85+
context: { testName: testName },
6986
crossDomain: crossDomain,
7087
data: "callback=?",
71-
dataType: "json"
72-
} ).catch( jQuery.noop );
73-
}
74-
);
75-
},
76-
77-
function() {
78-
return expectNoWarning( assert, "dataType: \"jsonp\", URL callback",
79-
function() {
88+
dataType: "json",
89+
jsonpCallback: "customJsonpCallback"
90+
} ).then( function() {
91+
assert.ok( patchEnabled, this.testName + " (success)" );
92+
} ).catch( function() {
93+
assert.ok( !patchEnabled, this.testName + " (failure)" );
94+
} );
95+
} );
96+
},
97+
98+
function() {
99+
var testName = "dataType: \"jsonp\", URL callback";
100+
return expectNoWarning( assert, testName, function() {
80101
return jQuery.ajax( {
81-
url: url( "null.json?callback=?" ),
102+
url: url( "jsonpScript.js?callback=?" ),
103+
context: { testName: testName },
82104
crossDomain: crossDomain,
83-
dataType: "jsonp"
84-
} ).catch( jQuery.noop );
85-
}
86-
);
87-
},
88-
89-
function() {
90-
return expectNoWarning( assert, "dataType: \"jsonp\", data callback",
91-
function() {
92-
return jQuery.ajax( {
93-
url: url( "null.json" ),
105+
dataType: "jsonp",
106+
jsonpCallback: "customJsonpCallback"
107+
} ).then( function() {
108+
assert.ok( true, this.testName + " (success)" );
109+
} ).catch( function() {
110+
assert.ok( false, this.testName + " (failure)" );
111+
} );
112+
} );
113+
},
114+
115+
function() {
116+
var testName = "dataType: \"jsonp\", data callback";
117+
return expectNoWarning( assert, testName, function() {
118+
return jQuery.ajax( {
119+
url: url( "jsonpScript.js" ),
120+
context: { testName: testName },
94121
crossDomain: crossDomain,
95122
data: "callback=?",
96-
dataType: "jsonp"
97-
} ).catch( jQuery.noop );
98-
}
99-
);
100-
}
101-
];
102-
103-
// Invoke tests sequentially as they're async and early tests could get warnings
104-
// from later ones.
105-
function run( tests ) {
106-
var test = tests[ 0 ];
107-
return test().then( function() {
108-
if ( tests.length > 1 ) {
109-
return run( tests.slice( 1 ) );
110-
}
111-
} );
112-
}
113-
114-
run( tests )
115-
.then( function() {
116-
done();
117-
} );
118-
} );
123+
dataType: "jsonp",
124+
jsonpCallback: "customJsonpCallback"
125+
} ).then( function() {
126+
assert.ok( true, this.testName + " (success)" );
127+
} ).catch( function() {
128+
assert.ok( false, this.testName + " (failure)" );
129+
} );
130+
} );
131+
}
132+
];
133+
134+
// Invoke tests sequentially as they're async and early tests could get warnings
135+
// from later ones.
136+
function run( tests ) {
137+
var test = tests[ 0 ];
138+
return test().then( function() {
139+
if ( tests.length > 1 ) {
140+
return run( tests.slice( 1 ) );
141+
}
142+
} );
143+
}
144+
145+
run( tests )
146+
.then( function() {
147+
done();
148+
} );
149+
} );
150+
}
151+
152+
if ( jQueryVersionSince( "4.0.0" ) ) {
153+
154+
// In jQuery 4+, this behavior is disabled by default for security
155+
// reasons, re-enable for this test, but test default behavior as well.
156+
runTests( { forceEnablePatch: true } );
157+
runTests( { forceEnablePatch: false } );
158+
} else {
159+
runTests();
160+
}
119161
} );
120162

121163
}

0 commit comments

Comments
 (0)