Ajax: Do not execute scripts for unsuccessful HTTP responses

skrobinson · mgol · commit 50871a5a85cc · 2019-09-26T02:43:30.000+02:00
The script transport used to evaluate fetched script sources which is undesirable for unsuccessful HTTP responses. This is different to other data types where such a convention was fine (e.g. in case of JSON). Fixes gh-4250 Closes gh-4379
diff --git a/src/ajax.js b/src/ajax.js
@@ -748,6 +748,11 @@ jQuery.extend( {
 				response = ajaxHandleResponses( s, jqXHR, responses );
 			}
 
+			// Use a noop converter for missing script
+			if ( !isSuccess && jQuery.inArray( "script", s.dataTypes ) > -1 ) {
+				s.converters[ "text script" ] = function() {};
+			}
+
 			// Convert no matter what (that way responseXXX fields are always set)
 			response = ajaxConvert( s, response, jqXHR, isSuccess );
 
diff --git a/test/data/mock.php b/test/data/mock.php
@@ -216,6 +216,19 @@ protected function cspClean( $req ) {
 		unlink( $this->cspFile );
 	}
 
+	protected function errorWithScript( $req ) {
+		header( 'HTTP/1.0 404 Not Found' );
+		if ( isset( $req->query['withScriptContentType'] ) ) {
+			header( 'Content-Type: application/javascript' );
+		}
+		if ( isset( $req->query['callback'] ) ) {
+			$callback = $req->query['callback'];
+			echo $callback . '( {"status": 404, "msg": "Not Found"} )';
+		} else {
+			echo 'QUnit.assert.ok( false, "Mock return erroneously executed" );';
+		}
+	}
+
 	public function __construct() {
 		$this->cspFile = __DIR__ . '/support/csp.log';
 	}
diff --git a/test/middleware-mockserver.js b/test/middleware-mockserver.js
@@ -226,6 +226,18 @@ var mocks = {
 		cspLog = "";
 		resp.writeHead( 200 );
 		resp.end();
+	},
+	errorWithScript: function( req, resp ) {
+		if ( req.query.withScriptContentType ) {
+			resp.writeHead( 404, { "Content-Type": "application/javascript" } );
+		} else {
+			resp.writeHead( 404 );
+		}
+		if ( req.query.callback ) {
+			resp.end( req.query.callback + "( {\"status\": 404, \"msg\": \"Not Found\"} )" );
+		} else {
+			resp.end( "QUnit.assert.ok( false, \"Mock return erroneously executed\" );" );
+		}
 	}
 };
 var handlers = {
diff --git a/test/unit/ajax.js b/test/unit/ajax.js
@@ -806,6 +806,121 @@ QUnit.module( "ajax", {
 		};
 	} );
 
+	ajaxTest( "jQuery.ajax() - do not execute scripts from unsuccessful responses (gh-4250)", 11, function( assert ) {
+		var globalEval = jQuery.globalEval;
+
+		var failConverters = {
+			"text script": function() {
+				assert.ok( false, "No converter for unsuccessful response" );
+			}
+		};
+
+		function request( title, options ) {
+			var testMsg = title + ": expected file missing status";
+			return jQuery.extend( {
+				beforeSend: function() {
+					jQuery.globalEval = function() {
+						assert.ok( false, "Should not eval" );
+					};
+				},
+				complete: function() {
+					jQuery.globalEval = globalEval;
+				},
+				// error is the significant assertion
+				error: function( xhr ) {
+					assert.strictEqual( xhr.status, 404, testMsg );
+				},
+				success: function() {
+					assert.ok( false, "Unanticipated success" );
+				}
+			}, options );
+		}
+
+		return [
+			request(
+				"HTML reply",
+				{
+					url: url( "404.txt" )
+				}
+			),
+			request(
+				"HTML reply with dataType",
+				{
+					dataType: "script",
+					url: url( "404.txt" )
+				}
+			),
+			request(
+				"script reply",
+				{
+					url: url( "mock.php?action=errorWithScript&withScriptContentType" )
+				}
+			),
+			request(
+				"non-script reply",
+				{
+					url: url( "mock.php?action=errorWithScript" )
+				}
+			),
+			request(
+				"script reply with dataType",
+				{
+					dataType: "script",
+					url: url( "mock.php?action=errorWithScript&withScriptContentType" )
+				}
+			),
+			request(
+				"non-script reply with dataType",
+				{
+					dataType: "script",
+					url: url( "mock.php?action=errorWithScript" )
+				}
+			),
+			request(
+				"script reply with converter",
+				{
+					converters: failConverters,
+					url: url( "mock.php?action=errorWithScript&withScriptContentType" )
+				}
+			),
+			request(
+				"non-script reply with converter",
+				{
+					converters: failConverters,
+					url: url( "mock.php?action=errorWithScript" )
+				}
+			),
+			request(
+				"script reply with converter and dataType",
+				{
+					converters: failConverters,
+					dataType: "script",
+					url: url( "mock.php?action=errorWithScript&withScriptContentType" )
+				}
+			),
+			request(
+				"non-script reply with converter and dataType",
+				{
+					converters: failConverters,
+					dataType: "script",
+					url: url( "mock.php?action=errorWithScript" )
+				}
+			),
+			request(
+				"JSONP reply with dataType",
+				{
+					dataType: "jsonp",
+					url: url( "mock.php?action=errorWithScript" ),
+					beforeSend: function() {
+						jQuery.globalEval = function( response ) {
+							assert.ok( /"status": 404, "msg": "Not Found"/.test( response ), "Error object returned" );
+						};
+					}
+				}
+			)
+		];
+	} );
+
 	ajaxTest( "jQuery.ajax() - synchronous request", 1, function( assert ) {
 		return {
 			url: url( "json_obj.js" ),
