From f193271cffbca25d01df6af3b33418b0721b1d5f Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Mon, 20 Feb 2017 16:29:54 +1100 Subject: [PATCH] Log a backtrace from the culprit location if headers were already sent Install the backtrace collector very early, so that we can get the backtrace even if headers were sent from LocalSettings.php. Bug: T157392 Change-Id: I9bc732b34481c95afb5362e135a87bd4302498e2 --- autoload.php | 1 + includes/GlobalFunctions.php | 1 + includes/HeaderCallback.php | 69 ++++++++++++++++++++++ includes/Setup.php | 29 --------- includes/WebResponse.php | 4 +- includes/WebStart.php | 3 + includes/libs/HttpStatus.php | 1 + includes/resourceloader/ResourceLoader.php | 1 + 8 files changed, 77 insertions(+), 32 deletions(-) create mode 100644 includes/HeaderCallback.php diff --git a/autoload.php b/autoload.php index b21310e04c..fce7891cd2 100644 --- a/autoload.php +++ b/autoload.php @@ -852,6 +852,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Auth\\UsernameAuthenticationRequest' => __DIR__ . '/includes/auth/UsernameAuthenticationRequest.php', 'MediaWiki\\Diff\\ComplexityException' => __DIR__ . '/includes/diff/ComplexityException.php', 'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/diff/WordAccumulator.php', + 'MediaWiki\\HeaderCallback' => __DIR__ . '/includes/HeaderCallback.php', 'MediaWiki\\Interwiki\\ClassicInterwikiLookup' => __DIR__ . '/includes/interwiki/ClassicInterwikiLookup.php', 'MediaWiki\\Interwiki\\InterwikiLookup' => __DIR__ . '/includes/interwiki/InterwikiLookup.php', 'MediaWiki\\Interwiki\\InterwikiLookupAdapter' => __DIR__ . '/includes/interwiki/InterwikiLookupAdapter.php', diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 46def53e0e..7a34c740ba 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -1788,6 +1788,7 @@ function wfHttpError( $code, $label, $desc ) { $wgOut->sendCacheControl(); } + MediaWiki\HeaderCallback::warnIfHeadersSent(); header( 'Content-type: text/html; charset=utf-8' ); print '' . '' . diff --git a/includes/HeaderCallback.php b/includes/HeaderCallback.php new file mode 100644 index 0000000000..b2ca6733f6 --- /dev/null +++ b/includes/HeaderCallback.php @@ -0,0 +1,69 @@ +<?php + +namespace MediaWiki; + +class HeaderCallback { + private static $headersSentException; + private static $messageSent = false; + + /** + * Register a callback to be called when headers are sent. There can only + * be one of these handlers active, so all relevant actions have to be in + * here. + */ + public static function register() { + header_register_callback( [ __CLASS__, 'callback' ] ); + } + + /** + * The callback, which is called by the transport + */ + public static function callback() { + // Prevent caching of responses with cookies (T127993) + $headers = []; + foreach ( headers_list() as $header ) { + list( $name, $value ) = explode( ':', $header, 2 ); + $headers[strtolower( trim( $name ) )][] = trim( $value ); + } + + if ( isset( $headers['set-cookie'] ) ) { + $cacheControl = isset( $headers['cache-control'] ) + ? implode( ', ', $headers['cache-control'] ) + : ''; + + if ( !preg_match( '/(?:^|,)\s*(?:private|no-cache|no-store)\s*(?:$|,)/i', + $cacheControl ) + ) { + header( 'Expires: Thu, 01 Jan 1970 00:00:00 GMT' ); + header( 'Cache-Control: private, max-age=0, s-maxage=0' ); + \MediaWiki\Logger\LoggerFactory::getInstance( 'cache-cookies' )->warning( + 'Cookies set on {url} with Cache-Control "{cache-control}"', [ + 'url' => \WebRequest::getGlobalRequestURL(), + 'cookies' => $headers['set-cookie'], + 'cache-control' => $cacheControl ?: '<not set>', + ] + ); + } + } + + // Save a backtrace for logging in case it turns out that headers were sent prematurely + self::$headersSentException = new \Exception( 'Headers already sent from this point' ); + } + + /** + * Log a warning message if headers have already been sent. This can be + * called before flushing the output. + */ + public static function warnIfHeadersSent() { + if ( headers_sent() && !self::$messageSent ) { + self::$messageSent = true; + \MWDebug::warning( 'Headers already sent, should send headers earlier than ' . + wfGetCaller( 3 ) ); + $logger = \MediaWiki\Logger\LoggerFactory::getInstance( 'headers-sent' ); + $logger->error( 'Warning: headers were already sent from the location below', [ + 'exception' => self::$headersSentException, + 'detection-trace' => new \Exception( 'Detected here' ), + ] ); + } + } +} diff --git a/includes/Setup.php b/includes/Setup.php index 01ba1e8422..72ed1fd2c9 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -521,35 +521,6 @@ if ( $wgSharedDB && $wgSharedTables ) { // is complete. define( 'MW_SERVICE_BOOTSTRAP_COMPLETE', 1 ); -// Install a header callback to prevent caching of responses with cookies (T127993) -if ( !$wgCommandLineMode ) { - header_register_callback( function () { - $headers = []; - foreach ( headers_list() as $header ) { - list( $name, $value ) = explode( ':', $header, 2 ); - $headers[strtolower( trim( $name ) )][] = trim( $value ); - } - - if ( isset( $headers['set-cookie'] ) ) { - $cacheControl = isset( $headers['cache-control'] ) - ? implode( ', ', $headers['cache-control'] ) - : ''; - - if ( !preg_match( '/(?:^|,)\s*(?:private|no-cache|no-store)\s*(?:$|,)/i', $cacheControl ) ) { - header( 'Expires: Thu, 01 Jan 1970 00:00:00 GMT' ); - header( 'Cache-Control: private, max-age=0, s-maxage=0' ); - MediaWiki\Logger\LoggerFactory::getInstance( 'cache-cookies' )->warning( - 'Cookies set on {url} with Cache-Control "{cache-control}"', [ - 'url' => WebRequest::getGlobalRequestURL(), - 'cookies' => $headers['set-cookie'], - 'cache-control' => $cacheControl ?: '<not set>', - ] - ); - } - } - } ); -} - MWExceptionHandler::installHandler(); require_once "$IP/includes/compat/normal/UtfNormalUtil.php"; diff --git a/includes/WebResponse.php b/includes/WebResponse.php index f5fb47fc5b..0208a72ab9 100644 --- a/includes/WebResponse.php +++ b/includes/WebResponse.php @@ -39,9 +39,7 @@ 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 ( headers_sent() ) { - MWDebug::warning( 'Headers already sent, should send headers earlier than ' . wfGetCaller() ); - } + \MediaWiki\HeaderCallback::warnIfHeadersSent(); if ( $http_response_code ) { header( $string, $replace, $http_response_code ); } else { diff --git a/includes/WebStart.php b/includes/WebStart.php index 6e4fb09de0..861e532a49 100644 --- a/includes/WebStart.php +++ b/includes/WebStart.php @@ -104,6 +104,9 @@ if ( !interface_exists( 'Psr\Log\LoggerInterface' ) ) { die( 1 ); } +# Install a header callback +MediaWiki\HeaderCallback::register(); + if ( defined( 'MW_CONFIG_CALLBACK' ) ) { # Use a callback function to configure MediaWiki call_user_func( MW_CONFIG_CALLBACK ); diff --git a/includes/libs/HttpStatus.php b/includes/libs/HttpStatus.php index 72fc333869..27f872857c 100644 --- a/includes/libs/HttpStatus.php +++ b/includes/libs/HttpStatus.php @@ -101,6 +101,7 @@ class HttpStatus { return false; } + MediaWiki\HeaderCallback::warnIfHeadersSent(); if ( $version === null ) { $version = isset( $_SERVER['SERVER_PROTOCOL'] ) && $_SERVER['SERVER_PROTOCOL'] === 'HTTP/1.0' ? diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index a55cbc1b2c..59c60dfc79 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -818,6 +818,7 @@ class ResourceLoader implements LoggerAwareInterface { * @return void */ protected function sendResponseHeaders( ResourceLoaderContext $context, $etag, $errors ) { + \MediaWiki\HeaderCallback::warnIfHeadersSent(); $rlMaxage = $this->config->get( 'ResourceLoaderMaxage' ); // Use a short cache expiry so that updates propagate to clients quickly, if: // - No version specified (shared resources, e.g. stylesheets) -- 2.20.1