From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Fri, 8 Jun 2018 18:51:25 +0000 (-0400)
Subject: Disable WebResponse setters for post-send processing
X-Git-Tag: 1.34.0-rc.0~5090^2
X-Git-Url: http://git.heureux-cyclage.org/?a=commitdiff_plain;h=23706be35c033f994d7bede56a27305f28416660;hp=b7116e4f7d53f7fc29020d4acb02971fe8b0c302;p=lhc%2Fweb%2Fwiklou.git

Disable WebResponse setters for post-send processing

When jobs are being run synchronously post-send, we don't want to allow
bugs to result in a job somehow setting cookies or headers that
interfere with those that were intended to be set in the request.

Bug: T191537
Change-Id: Ib5714a17af417797140f99e41eaacbba1bfd20f4
---

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 79787272eb..7fb59d5639 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -723,6 +723,9 @@ class MediaWiki {
 			MWExceptionHandler::rollbackMasterChangesAndLog( $e );
 		}
 
+		// Disable WebResponse setters for post-send processing (T191537).
+		WebResponse::disableForPostSend();
+
 		$blocksHttpClient = true;
 		// Defer everything else if possible...
 		$callback = function () use ( $mode, &$blocksHttpClient ) {
diff --git a/includes/WebResponse.php b/includes/WebResponse.php
index 0208a72ab9..82fe99efe6 100644
--- a/includes/WebResponse.php
+++ b/includes/WebResponse.php
@@ -32,6 +32,22 @@ class WebResponse {
 	 */
 	protected static $setCookies = [];
 
+	/** @var bool Used to disable setters before running jobs post-request (T191537) */
+	protected static $disableForPostSend = false;
+
+	/**
+	 * Disable setters for post-send processing
+	 *
+	 * After this call, self::setCookie(), self::header(), and
+	 * self::statusHeader() will log a warning and return without
+	 * setting cookies or headers.
+	 *
+	 * @since 1.32
+	 */
+	public static function disableForPostSend() {
+		self::$disableForPostSend = true;
+	}
+
 	/**
 	 * Output an HTTP header, wrapper for PHP's header()
 	 * @param string $string Header to output
@@ -39,6 +55,16 @@ class WebResponse {
 	 * @param null|int $http_response_code Forces the HTTP response code to the specified value.
 	 */
 	public function header( $string, $replace = true, $http_response_code = null ) {
+		if ( self::$disableForPostSend ) {
+			wfDebugLog( 'header', 'ignored post-send header {header}', 'all', [
+				'header' => $string,
+				'replace' => $replace,
+				'http_response_code' => $http_response_code,
+				'exception' => new RuntimeException( 'Ignored post-send header' ),
+			] );
+			return;
+		}
+
 		\MediaWiki\HeaderCallback::warnIfHeadersSent();
 		if ( $http_response_code ) {
 			header( $string, $replace, $http_response_code );
@@ -69,6 +95,14 @@ class WebResponse {
 	 * @param int $code Status code
 	 */
 	public function statusHeader( $code ) {
+		if ( self::$disableForPostSend ) {
+			wfDebugLog( 'header', 'ignored post-send status header {code}', 'all', [
+				'code' => $code,
+				'exception' => new RuntimeException( 'Ignored post-send status header' ),
+			] );
+			return;
+		}
+
 		HttpStatus::header( $code );
 	}
 
@@ -117,20 +151,29 @@ class WebResponse {
 			$expire = time() + $wgCookieExpiration;
 		}
 
+		$cookie = $options['prefix'] . $name;
+		$data = [
+			'name' => (string)$cookie,
+			'value' => (string)$value,
+			'expire' => (int)$expire,
+			'path' => (string)$options['path'],
+			'domain' => (string)$options['domain'],
+			'secure' => (bool)$options['secure'],
+			'httpOnly' => (bool)$options['httpOnly'],
+		];
+
+		if ( self::$disableForPostSend ) {
+			wfDebugLog( 'cookie', 'ignored post-send cookie {cookie}', 'all', [
+				'cookie' => $cookie,
+				'data' => $data,
+				'exception' => new RuntimeException( 'Ignored post-send cookie' ),
+			] );
+			return;
+		}
+
 		$func = $options['raw'] ? 'setrawcookie' : 'setcookie';
 
 		if ( Hooks::run( 'WebResponseSetCookie', [ &$name, &$value, &$expire, &$options ] ) ) {
-			$cookie = $options['prefix'] . $name;
-			$data = [
-				'name' => (string)$cookie,
-				'value' => (string)$value,
-				'expire' => (int)$expire,
-				'path' => (string)$options['path'],
-				'domain' => (string)$options['domain'],
-				'secure' => (bool)$options['secure'],
-				'httpOnly' => (bool)$options['httpOnly'],
-			];
-
 			// Per RFC 6265, key is name + domain + path
 			$key = "{$data['name']}\n{$data['domain']}\n{$data['path']}";
 
diff --git a/tests/phpunit/includes/MediaWikiTest.php b/tests/phpunit/includes/MediaWikiTest.php
index a8d1e33950..7d7e637673 100644
--- a/tests/phpunit/includes/MediaWikiTest.php
+++ b/tests/phpunit/includes/MediaWikiTest.php
@@ -154,4 +154,53 @@ class MediaWikiTest extends MediaWikiTestCase {
 			$context->getOutput()->getRedirect()
 		);
 	}
+
+	/**
+	 * Test a post-send job can not set cookies (T191537).
+	 */
+	public function testPostSendJobDoesNotSetCookie() {
+		// Prevent updates from running
+		$this->setMwGlobals( 'wgCommandLineMode', false );
+
+		$response = new WebResponse;
+
+		// A job that attempts to set a cookie
+		$jobHasRun = false;
+		DeferredUpdates::addCallableUpdate( function () use ( $response, &$jobHasRun ) {
+			$jobHasRun = true;
+			$response->setCookie( 'JobCookie', 'yes' );
+			$response->header( 'Foo: baz' );
+		} );
+
+		$hookWasRun = false;
+		$this->setTemporaryHook( 'WebResponseSetCookie', function () use ( &$hookWasRun ) {
+			$hookWasRun = true;
+			return true;
+		} );
+
+		$logger = new TestLogger();
+		$logger->setCollect( true );
+		$this->setLogger( 'cookie', $logger );
+		$this->setLogger( 'header', $logger );
+
+		$mw = new MediaWiki();
+		$mw->doPostOutputShutdown();
+		// restInPeace() might have been registered to a callback of
+		// register_postsend_function() and thus can not be triggered from
+		// PHPUnit.
+		if ( $jobHasRun === false ) {
+			$mw->restInPeace();
+		}
+
+		$this->assertTrue( $jobHasRun, 'post-send job has run' );
+		$this->assertFalse( $hookWasRun,
+			'post-send job must not trigger WebResponseSetCookie hook' );
+		$this->assertEquals(
+			[
+				[ 'info', 'ignored post-send cookie {cookie}' ],
+				[ 'info', 'ignored post-send header {header}' ],
+			],
+			$logger->getBuffer()
+		);
+	}
 }