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 7978727..7fb59d5 100644 (file)
--- 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 0208a72..82fe99e 100644 (file)
--- 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 a8d1e33..7d7e637 100644 (file)
--- 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()
+               );
+       }
 }