Ajax: Avoid CSP errors in the script transport for async requests

mgol · web-flow · commit 07a8e4a17755 · 2020-08-25T21:28:30.000+02:00
Until now, the AJAX script transport only used a script tag to load scripts for cross-domain requests or ones with `scriptAttrs` set. This commit makes it also used for all async requests to avoid CSP errors arising from usage of inline scripts. This also makes `jQuery.getScript` not trigger CSP errors as it uses the AJAX script transport under the hood. For sync requests such a change is impossible and that's what `jQuery._evalUrl` uses. Fixing that is tracked in gh-1895. The commit also makes other type of requests using the script tag version of the script transport set its type to "GET", namely async scripts & ones with `scriptAttrs` set in addition to the existing cross-domain ones. Fixes gh-3969 Closes gh-4763
diff --git a/src/ajax/script.js b/src/ajax/script.js
@@ -32,16 +32,20 @@ jQuery.ajaxPrefilter( "script", function( s ) {
 	if ( s.cache === undefined ) {
 		s.cache = false;
 	}
-	if ( s.crossDomain ) {
+
+	// These types of requests are handled via a script tag
+	// so force their methods to GET.
+	if ( s.crossDomain || s.async || s.scriptAttrs ) {
 		s.type = "GET";
 	}
 } );
 
 // Bind script tag hack transport
 jQuery.ajaxTransport( "script", function( s ) {
 
-	// This transport only deals with cross domain or forced-by-attrs requests
-	if ( s.crossDomain || s.scriptAttrs ) {
+	// This transport only deals with async, cross domain or forced-by-attrs requests.
+	// Sync requests remain handled differently to preserve strict script ordering.
+	if ( s.crossDomain || s.async || s.scriptAttrs ) {
 		var script, callback;
 		return {
 			send: function( _, complete ) {
diff --git a/test/data/csp-ajax-script-downloaded.js b/test/data/csp-ajax-script-downloaded.js
@@ -0,0 +1 @@
+window.downloadedScriptCalled = true;
diff --git a/test/data/csp-ajax-script.html b/test/data/csp-ajax-script.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+	<title>jQuery.ajax() - script, CSP script-src compat (gh-3969)</title>
+	<script src="../jquery.js"></script>
+	<script src="iframeTest.js"></script>
+	<script src="csp-ajax-script.js"></script>
+</head>
+<body>
+	<p>CSP Test Page</p>
+</body>
+</html>
diff --git a/test/data/csp-ajax-script.js b/test/data/csp-ajax-script.js
@@ -0,0 +1,25 @@
+/* global startIframeTest */
+
+var timeoutId, type;
+
+function finalize() {
+	startIframeTest( type, window.downloadedScriptCalled );
+}
+
+timeoutId = setTimeout( function() {
+	finalize();
+}, 1000 );
+
+jQuery
+	.ajax( {
+		url: "csp-ajax-script-downloaded.js",
+		dataType: "script",
+		method: "POST",
+		beforeSend: function( _jqXhr, settings ) {
+			type = settings.type;
+		}
+	} )
+	.then( function() {
+		clearTimeout( timeoutId );
+		finalize();
+	} );
diff --git a/test/data/mock.php b/test/data/mock.php
@@ -195,22 +195,24 @@ protected function testHTML( $req ) {
 	}
 
 	protected function cspFrame( $req ) {
-		// This is CSP only for browsers with "Content-Security-Policy" header support
-		// i.e. no old WebKit or old Firefox
 		header( "Content-Security-Policy: default-src 'self'; report-uri ./mock.php?action=cspLog" );
 		header( 'Content-type: text/html' );
 		echo file_get_contents( __DIR__ . '/csp.include.html' );
 	}
 
 	protected function cspNonce( $req ) {
-		// This is CSP only for browsers with "Content-Security-Policy" header support
-		// i.e. no old WebKit or old Firefox
 		$test = $req->query['test'] ? '-' . $req->query['test'] : '';
 		header( "Content-Security-Policy: script-src 'nonce-jquery+hardcoded+nonce'; report-uri ./mock.php?action=cspLog" );
 		header( 'Content-type: text/html' );
 		echo file_get_contents( __DIR__ . '/csp-nonce' . $test . '.html' );
 	}
 
+	protected function cspAjaxScript( $req ) {
+		header( "Content-Security-Policy: script-src 'self'; report-uri /base/test/data/mock.php?action=cspLog" );
+		header( 'Content-type: text/html' );
+		echo file_get_contents( __DIR__ . '/csp-ajax-script.html' );
+	}
+
 	protected function cspLog( $req ) {
 		file_put_contents( $this->cspFile, 'error' );
 	}
diff --git a/test/middleware-mockserver.js b/test/middleware-mockserver.js
@@ -222,6 +222,15 @@ var mocks = {
 			__dirname + "/data/csp-nonce" + testParam + ".html" ).toString();
 		resp.end( body );
 	},
+	cspAjaxScript: function( req, resp ) {
+		resp.writeHead( 200, {
+			"Content-Type": "text/html",
+			"Content-Security-Policy": "script-src 'self'; report-uri /base/test/data/mock.php?action=cspLog"
+		} );
+		var body = fs.readFileSync(
+			__dirname + "/data/csp-ajax-script.html" ).toString();
+		resp.end( body );
+	},
 	cspLog: function( req, resp ) {
 		cspLog = "error";
 		resp.writeHead( 200 );
diff --git a/test/unit/ajax.js b/test/unit/ajax.js
@@ -89,17 +89,21 @@ QUnit.module( "ajax", {
 		}
 	);
 
-	ajaxTest( "jQuery.ajax() - custom attributes for script tag", 4,
+	ajaxTest( "jQuery.ajax() - custom attributes for script tag", 5,
 		function( assert ) {
 			return {
 				create: function( options ) {
 					var xhr;
+					options.method = "POST";
 					options.dataType = "script";
 					options.scriptAttrs = { id: "jquery-ajax-test", async: "async" };
 					xhr = jQuery.ajax( url( "mock.php?action=script" ), options );
 					assert.equal( jQuery( "#jquery-ajax-test" ).attr( "async" ), "async", "attr value" );
 					return xhr;
 				},
+				beforeSend: function( _jqXhr, settings ) {
+					assert.strictEqual( settings.type, "GET", "Type changed to GET" );
+				},
 				success: function() {
 					assert.ok( true, "success" );
 				},
@@ -1356,6 +1360,17 @@ QUnit.module( "ajax", {
 
 	} );
 
+	testIframe(
+		"jQuery.ajax() - script, CSP script-src compat (gh-3969)",
+		"mock.php?action=cspAjaxScript",
+		function( assert, jQuery, window, document, type, downloadedScriptCalled ) {
+			assert.expect( 2 );
+
+			assert.strictEqual( type, "GET", "Type changed to GET" );
+			assert.strictEqual( downloadedScriptCalled, true, "External script called" );
+		}
+	);
+
 	ajaxTest( "jQuery.ajax() - script, Remote", 2, function( assert ) {
 		return {
 			setup: function() {
@@ -1369,12 +1384,15 @@ QUnit.module( "ajax", {
 		};
 	} );
 
-	ajaxTest( "jQuery.ajax() - script, Remote with POST", 3, function( assert ) {
+	ajaxTest( "jQuery.ajax() - script, Remote with POST", 4, function( assert ) {
 		return {
 			setup: function() {
 				Globals.register( "testBar" );
 			},
 			url: url( "mock.php?action=testbar" ),
+			beforeSend: function( _jqXhr, settings ) {
+				assert.strictEqual( settings.type, "GET", "Type changed to GET" );
+			},
 			type: "POST",
 			dataType: "script",
 			success: function( data, status ) {
