From 665e9b7bf26f9eccdfe02b1a1889f70e8d105705 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sat, 17 Mar 2018 21:03:42 -0700 Subject: [PATCH] Convert OutputHandler functions to a class MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Convert OutputHandler.php from global functions to a class. - wfOutputHandler → OutputHandler::handle (no alias, no usage outside core) - wfGzipHandler → OutputHandler::handleGzip (private, no usage outside class) - wfRequestExtension → OutputHandler::findUriExtension (private, no usage outside class) - wfMangleFlashPolicy → OutputHandler::mangleFlashPolicy (private, no usage outside class) - wfDoContentLength → OutputHandler::emitContentLength (private, no usage outside class) - wfHtmlValidationHandler → OutputHandler::validateAllHtml (private, no usage outside class) * Add the class to autoload.php for exposure outside WebStart. Specifically, for use in ApiFormatPhpTest. This also removes the need to manually load the class because this code runs after Setup.php loads AutoLoader.php. Bug: T189966 Change-Id: I27a41ec0ae0ee30aeb313a616323b967605c4055 --- RELEASE-NOTES-1.31 | 5 +- autoload.php | 1 + includes/GlobalFunctions.php | 2 +- includes/OutputHandler.php | 375 +++++++++--------- includes/WebStart.php | 10 +- includes/api/ApiFormatBase.php | 2 +- includes/api/ApiFormatJson.php | 2 +- includes/api/ApiFormatPhp.php | 4 +- .../includes/api/format/ApiFormatPhpTest.php | 10 +- 9 files changed, 207 insertions(+), 204 deletions(-) diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31 index 899f8b3d95..48b05001e4 100644 --- a/RELEASE-NOTES-1.31 +++ b/RELEASE-NOTES-1.31 @@ -287,7 +287,7 @@ changes to languages because of Phabricator reports. * Parser::isValidHalfParsedText() * StripState::getSubState() * StripState::merge() -* The "free" class is now only applied to unbracketed URLs in wikitext. Links +* The "free" CSS class is now only applied to unbracketed URLs in wikitext. Links written using square brackets will get the class "text" not "free". * OpenSearch::getOpenSearchTemplate(), deprecated in 1.25, has been removed. You can use ApiOpenSearch::getOpenSearchTemplate() instead. @@ -302,6 +302,9 @@ changes to languages because of Phabricator reports. transaction also results in an exception. Previously these were logged as errors. The startAtomic() and endAtomic() methods, or AtomicSectionUpdate should be used instead. +* The global function wfOutputHandler() was removed, use the its replacement + MediaWiki\OutputHandler::handle() instead. The global function was only sometimes defined. + Its replacement is always available via the autoloader. == Compatibility == MediaWiki 1.31 requires PHP 5.5.9 or later. Although HHVM 3.18.5 or later is supported, diff --git a/autoload.php b/autoload.php index b5f3e4a067..e61eec2ca0 100644 --- a/autoload.php +++ b/autoload.php @@ -911,6 +911,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Logger\\NullSpi' => __DIR__ . '/includes/debug/logger/NullSpi.php', 'MediaWiki\\Logger\\Spi' => __DIR__ . '/includes/debug/logger/Spi.php', 'MediaWiki\\MediaWikiServices' => __DIR__ . '/includes/MediaWikiServices.php', + 'MediaWiki\\OutputHandler' => __DIR__ . '/includes/OutputHandler.php', 'MediaWiki\\Preferences\\DefaultPreferencesFactory' => __DIR__ . '/includes/preferences/DefaultPreferencesFactory.php', 'MediaWiki\\Preferences\\PreferencesFactory' => __DIR__ . '/includes/preferences/PreferencesFactory.php', 'MediaWiki\\ProcOpenError' => __DIR__ . '/includes/exception/ProcOpenError.php', diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 7a5a5d8489..42a6573257 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -1845,7 +1845,7 @@ function wfHttpError( $code, $label, $desc ) { function wfResetOutputBuffers( $resetGzipEncoding = true ) { if ( $resetGzipEncoding ) { // Suppress Content-Encoding and Content-Length - // headers from 1.10+s wfOutputHandler + // headers from OutputHandler::handle. global $wgDisableOutputCompression; $wgDisableOutputCompression = true; } diff --git a/includes/OutputHandler.php b/includes/OutputHandler.php index 2dc3732011..842922d566 100644 --- a/includes/OutputHandler.php +++ b/includes/OutputHandler.php @@ -20,219 +20,228 @@ * @file */ +namespace MediaWiki; + +use MWTidy; +use Html; + /** - * Standard output handler for use with ob_start - * - * @param string $s - * - * @return string + * @since 1.31 */ -function wfOutputHandler( $s ) { - global $wgDisableOutputCompression, $wgValidateAllHtml, $wgMangleFlashPolicy; - if ( $wgMangleFlashPolicy ) { - $s = wfMangleFlashPolicy( $s ); - } - if ( $wgValidateAllHtml ) { - $headers = headers_list(); - $isHTML = false; - foreach ( $headers as $header ) { - $parts = explode( ':', $header, 2 ); - if ( count( $parts ) !== 2 ) { - continue; +class OutputHandler { + /** + * Standard output handler for use with ob_start. + * + * @param string $s Web response output + * @return string + */ + public static function handle( $s ) { + global $wgDisableOutputCompression, $wgValidateAllHtml, $wgMangleFlashPolicy; + if ( $wgMangleFlashPolicy ) { + $s = self::mangleFlashPolicy( $s ); + } + if ( $wgValidateAllHtml ) { + $headers = headers_list(); + $isHTML = false; + foreach ( $headers as $header ) { + $parts = explode( ':', $header, 2 ); + if ( count( $parts ) !== 2 ) { + continue; + } + $name = strtolower( trim( $parts[0] ) ); + $value = trim( $parts[1] ); + if ( $name == 'content-type' && ( strpos( $value, 'text/html' ) === 0 + || strpos( $value, 'application/xhtml+xml' ) === 0 ) + ) { + $isHTML = true; + break; + } } - $name = strtolower( trim( $parts[0] ) ); - $value = trim( $parts[1] ); - if ( $name == 'content-type' && ( strpos( $value, 'text/html' ) === 0 - || strpos( $value, 'application/xhtml+xml' ) === 0 ) - ) { - $isHTML = true; - break; + if ( $isHTML ) { + $s = self::validateAllHtml( $s ); } } - if ( $isHTML ) { - $s = wfHtmlValidationHandler( $s ); + if ( !$wgDisableOutputCompression && !ini_get( 'zlib.output_compression' ) ) { + if ( !defined( 'MW_NO_OUTPUT_COMPRESSION' ) ) { + $s = self::handleGzip( $s ); + } + if ( !ini_get( 'output_handler' ) ) { + self::emitContentLength( strlen( $s ) ); + } } + return $s; } - if ( !$wgDisableOutputCompression && !ini_get( 'zlib.output_compression' ) ) { - if ( !defined( 'MW_NO_OUTPUT_COMPRESSION' ) ) { - $s = wfGzipHandler( $s ); - } - if ( !ini_get( 'output_handler' ) ) { - wfDoContentLength( strlen( $s ) ); + + /** + * Get the "file extension" that some client apps will estimate from + * the currently-requested URL. + * + * This isn't a WebRequest method, because we need it before the class loads. + * @todo As of 2018, this actually runs after autoloader in Setup.php, so + * WebRequest seems like a good place for this. + * + * @return string + */ + private static function findUriExtension() { + /// @todo FIXME: this sort of dupes some code in WebRequest::getRequestUrl() + if ( isset( $_SERVER['REQUEST_URI'] ) ) { + // Strip the query string... + list( $path ) = explode( '?', $_SERVER['REQUEST_URI'], 2 ); + } elseif ( isset( $_SERVER['SCRIPT_NAME'] ) ) { + // Probably IIS. QUERY_STRING appears separately. + $path = $_SERVER['SCRIPT_NAME']; + } else { + // Can't get the path from the server? :( + return ''; } - } - return $s; -} -/** - * Get the "file extension" that some client apps will estimate from - * the currently-requested URL. - * This isn't on WebRequest because we need it when things aren't initialized - * @private - * - * @return string - */ -function wfRequestExtension() { - /// @todo FIXME: this sort of dupes some code in WebRequest::getRequestUrl() - if ( isset( $_SERVER['REQUEST_URI'] ) ) { - // Strip the query string... - list( $path ) = explode( '?', $_SERVER['REQUEST_URI'], 2 ); - } elseif ( isset( $_SERVER['SCRIPT_NAME'] ) ) { - // Probably IIS. QUERY_STRING appears separately. - $path = $_SERVER['SCRIPT_NAME']; - } else { - // Can't get the path from the server? :( + $period = strrpos( $path, '.' ); + if ( $period !== false ) { + return strtolower( substr( $path, $period ) ); + } return ''; } - $period = strrpos( $path, '.' ); - if ( $period !== false ) { - return strtolower( substr( $path, $period ) ); - } - return ''; -} - -/** - * Handler that compresses data with gzip if allowed by the Accept header. - * Unlike ob_gzhandler, it works for HEAD requests too. - * - * @param string $s - * - * @return string - */ -function wfGzipHandler( $s ) { - if ( !function_exists( 'gzencode' ) ) { - wfDebug( __FUNCTION__ . "() skipping compression (gzencode unavailable)\n" ); - return $s; - } - if ( headers_sent() ) { - wfDebug( __FUNCTION__ . "() skipping compression (headers already sent)\n" ); - return $s; - } + /** + * Handler that compresses data with gzip if allowed by the Accept header. + * + * Unlike ob_gzhandler, it works for HEAD requests too. + * + * @param string $s Web response output + * @return string + */ + private static function handleGzip( $s ) { + if ( !function_exists( 'gzencode' ) ) { + wfDebug( __METHOD__ . "() skipping compression (gzencode unavailable)\n" ); + return $s; + } + if ( headers_sent() ) { + wfDebug( __METHOD__ . "() skipping compression (headers already sent)\n" ); + return $s; + } - $ext = wfRequestExtension(); - if ( $ext == '.gz' || $ext == '.tgz' ) { - // Don't do gzip compression if the URL path ends in .gz or .tgz - // This confuses Safari and triggers a download of the page, - // even though it's pretty clearly labeled as viewable HTML. - // Bad Safari! Bad! - return $s; - } + $ext = self::findUriExtension(); + if ( $ext == '.gz' || $ext == '.tgz' ) { + // Don't do gzip compression if the URL path ends in .gz or .tgz + // This confuses Safari and triggers a download of the page, + // even though it's pretty clearly labeled as viewable HTML. + // Bad Safari! Bad! + return $s; + } - if ( wfClientAcceptsGzip() ) { - wfDebug( __FUNCTION__ . "() is compressing output\n" ); - header( 'Content-Encoding: gzip' ); - $s = gzencode( $s, 6 ); - } + if ( wfClientAcceptsGzip() ) { + wfDebug( __METHOD__ . "() is compressing output\n" ); + header( 'Content-Encoding: gzip' ); + $s = gzencode( $s, 6 ); + } - // Set vary header if it hasn't been set already - $headers = headers_list(); - $foundVary = false; - foreach ( $headers as $header ) { - $headerName = strtolower( substr( $header, 0, 5 ) ); - if ( $headerName == 'vary:' ) { - $foundVary = true; - break; + // Set vary header if it hasn't been set already + $headers = headers_list(); + $foundVary = false; + foreach ( $headers as $header ) { + $headerName = strtolower( substr( $header, 0, 5 ) ); + if ( $headerName == 'vary:' ) { + $foundVary = true; + break; + } } - } - if ( !$foundVary ) { - header( 'Vary: Accept-Encoding' ); - global $wgUseKeyHeader; - if ( $wgUseKeyHeader ) { - header( 'Key: Accept-Encoding;match=gzip' ); + if ( !$foundVary ) { + header( 'Vary: Accept-Encoding' ); + global $wgUseKeyHeader; + if ( $wgUseKeyHeader ) { + header( 'Key: Accept-Encoding;match=gzip' ); + } } - } - return $s; -} - -/** - * Mangle flash policy tags which open up the site to XSS attacks. - * - * @param string $s - * - * @return string - */ -function wfMangleFlashPolicy( $s ) { - # Avoid weird excessive memory usage in PCRE on big articles - if ( preg_match( '/\<\s*cross-domain-policy(?=\s|\>)/i', $s ) ) { - return preg_replace( '/\<(\s*)(cross-domain-policy(?=\s|\>))/i', '<$1NOT-$2', $s ); - } else { return $s; } -} -/** - * Add a Content-Length header if possible. This makes it cooperate with CDN better. - * - * @param int $length - */ -function wfDoContentLength( $length ) { - if ( !headers_sent() - && isset( $_SERVER['SERVER_PROTOCOL'] ) - && $_SERVER['SERVER_PROTOCOL'] == 'HTTP/1.0' - ) { - header( "Content-Length: $length" ); + /** + * Mangle flash policy tags which open up the site to XSS attacks. + * + * @param string $s Web response output + * @return string + */ + private static function mangleFlashPolicy( $s ) { + # Avoid weird excessive memory usage in PCRE on big articles + if ( preg_match( '/\<\s*cross-domain-policy(?=\s|\>)/i', $s ) ) { + return preg_replace( '/\<(\s*)(cross-domain-policy(?=\s|\>))/i', '<$1NOT-$2', $s ); + } else { + return $s; + } } -} -/** - * Replace the output with an error if the HTML is not valid - * - * @param string $s - * - * @return string - */ -function wfHtmlValidationHandler( $s ) { - $errors = ''; - if ( MWTidy::checkErrors( $s, $errors ) ) { - return $s; + /** + * Add a Content-Length header if possible. This makes it cooperate with CDN better. + * + * @param int $length + */ + private static function emitContentLength( $length ) { + if ( !headers_sent() + && isset( $_SERVER['SERVER_PROTOCOL'] ) + && $_SERVER['SERVER_PROTOCOL'] == 'HTTP/1.0' + ) { + header( "Content-Length: $length" ); + } } - header( 'Cache-Control: no-cache' ); + /** + * Replace the output with an error if the HTML is not valid. + * + * @param string $s + * @return string + */ + private static function validateAllHtml( $s ) { + $errors = ''; + if ( MWTidy::checkErrors( $s, $errors ) ) { + return $s; + } + + header( 'Cache-Control: no-cache' ); - $out = Html::element( 'h1', null, 'HTML validation error' ); - $out .= Html::openElement( 'ul' ); + $out = Html::element( 'h1', null, 'HTML validation error' ); + $out .= Html::openElement( 'ul' ); - $error = strtok( $errors, "\n" ); - $badLines = []; - while ( $error !== false ) { - if ( preg_match( '/^line (\d+)/', $error, $m ) ) { - $lineNum = intval( $m[1] ); - $badLines[$lineNum] = true; - $out .= Html::rawElement( 'li', null, - Html::element( 'a', [ 'href' => "#line-{$lineNum}" ], $error ) ) . "\n"; + $error = strtok( $errors, "\n" ); + $badLines = []; + while ( $error !== false ) { + if ( preg_match( '/^line (\d+)/', $error, $m ) ) { + $lineNum = intval( $m[1] ); + $badLines[$lineNum] = true; + $out .= Html::rawElement( 'li', null, + Html::element( 'a', [ 'href' => "#line-{$lineNum}" ], $error ) ) . "\n"; + } + $error = strtok( "\n" ); } - $error = strtok( "\n" ); - } - $out .= Html::closeElement( 'ul' ); - $out .= Html::element( 'pre', null, $errors ); - $out .= Html::openElement( 'ol' ) . "\n"; - $line = strtok( $s, "\n" ); - $i = 1; - while ( $line !== false ) { - $attrs = []; - if ( isset( $badLines[$i] ) ) { - $attrs['class'] = 'highlight'; - $attrs['id'] = "line-$i"; - } - $out .= Html::element( 'li', $attrs, $line ) . "\n"; - $line = strtok( "\n" ); - $i++; - } - $out .= Html::closeElement( 'ol' ); + $out .= Html::closeElement( 'ul' ); + $out .= Html::element( 'pre', null, $errors ); + $out .= Html::openElement( 'ol' ) . "\n"; + $line = strtok( $s, "\n" ); + $i = 1; + while ( $line !== false ) { + $attrs = []; + if ( isset( $badLines[$i] ) ) { + $attrs['class'] = 'highlight'; + $attrs['id'] = "line-$i"; + } + $out .= Html::element( 'li', $attrs, $line ) . "\n"; + $line = strtok( "\n" ); + $i++; + } + $out .= Html::closeElement( 'ol' ); - $style = << 'en', 'dir' => 'ltr' ] ) . - Html::rawElement( 'head', null, - Html::element( 'title', null, 'HTML validation error' ) . - Html::inlineStyle( $style ) ) . - Html::rawElement( 'body', null, $out ) . - Html::closeElement( 'html' ); + $out = Html::htmlHeader( [ 'lang' => 'en', 'dir' => 'ltr' ] ) . + Html::rawElement( 'head', null, + Html::element( 'title', null, 'HTML validation error' ) . + Html::inlineStyle( $style ) ) . + Html::rawElement( 'body', null, $out ) . + Html::closeElement( 'html' ); - return $out; + return $out; + } } diff --git a/includes/WebStart.php b/includes/WebStart.php index be95779af3..c9aecce4e0 100644 --- a/includes/WebStart.php +++ b/includes/WebStart.php @@ -78,14 +78,10 @@ if ( !defined( 'MW_CONFIG_CALLBACK' ) ) { // Custom setup for WebStart entry point if ( !defined( 'MW_SETUP_CALLBACK' ) ) { function wfWebStartSetup() { - # Initialise output buffering - # Check that there is no previous output or previously set up buffers, because - # that would cause us to potentially mix gzip and non-gzip output, creating a - # big mess. - global $IP; + // Initialise output buffering + // Check for previously set up buffers, to avoid a mix of gzip and non-gzip output. if ( ob_get_level() == 0 ) { - require_once "$IP/includes/OutputHandler.php"; - ob_start( 'wfOutputHandler' ); + ob_start( 'MediaWiki\\OutputHandler::handle' ); } } define( 'MW_SETUP_CALLBACK', 'wfWebStartSetup' ); diff --git a/includes/api/ApiFormatBase.php b/includes/api/ApiFormatBase.php index 4b93b31991..234fcfdd5e 100644 --- a/includes/api/ApiFormatBase.php +++ b/includes/api/ApiFormatBase.php @@ -315,7 +315,7 @@ abstract class ApiFormatBase extends ApiBase { false, FormatJson::ALL_OK ); - // T68776: wfMangleFlashPolicy() is needed to avoid a nasty bug in + // T68776: OutputHandler::mangleFlashPolicy() avoids a nasty bug in // Flash, but what it does isn't friendly for the API, so we need to // work around it. if ( preg_match( '/\<\s*cross-domain-policy\s*\>/i', $json ) ) { diff --git a/includes/api/ApiFormatJson.php b/includes/api/ApiFormatJson.php index 6d8e74309f..1aa9e159a3 100644 --- a/includes/api/ApiFormatJson.php +++ b/includes/api/ApiFormatJson.php @@ -87,7 +87,7 @@ class ApiFormatJson extends ApiFormatBase { $data = $this->getResult()->getResultData( null, $transform ); $json = FormatJson::encode( $data, $this->getIsHtml(), $opt ); - // T68776: wfMangleFlashPolicy() is needed to avoid a nasty bug in + // T68776: OutputHandler::mangleFlashPolicy() avoids a nasty bug in // Flash, but what it does isn't friendly for the API, so we need to // work around it. if ( preg_match( '/\<\s*cross-domain-policy(?=\s|\>)/i', $json ) ) { diff --git a/includes/api/ApiFormatPhp.php b/includes/api/ApiFormatPhp.php index b05097afe3..cc0f159eef 100644 --- a/includes/api/ApiFormatPhp.php +++ b/includes/api/ApiFormatPhp.php @@ -56,12 +56,12 @@ class ApiFormatPhp extends ApiFormatBase { } $text = serialize( $this->getResult()->getResultData( null, $transforms ) ); - // T68776: wfMangleFlashPolicy() is needed to avoid a nasty bug in + // T68776: OutputHandler::mangleFlashPolicy() avoids a nasty bug in // Flash, but what it does isn't friendly for the API. There's nothing // we can do here that isn't actively broken in some manner, so let's // just be broken in a useful manner. if ( $this->getConfig()->get( 'MangleFlashPolicy' ) && - in_array( 'wfOutputHandler', ob_list_handlers(), true ) && + in_array( 'MediaWiki\\OutputHandler::handle', ob_list_handlers(), true ) && preg_match( '/\<\s*cross-domain-policy(?=\s|\>)/i', $text ) ) { $this->dieWithError( 'apierror-formatphp', 'internalerror' ); diff --git a/tests/phpunit/includes/api/format/ApiFormatPhpTest.php b/tests/phpunit/includes/api/format/ApiFormatPhpTest.php index 2d2e29b90d..66e620e89a 100644 --- a/tests/phpunit/includes/api/format/ApiFormatPhpTest.php +++ b/tests/phpunit/includes/api/format/ApiFormatPhpTest.php @@ -110,14 +110,8 @@ class ApiFormatPhpTest extends ApiFormatTestBase { $main = new ApiMain( $context ); $main->getResult()->addValue( null, null, '< Cross-Domain-Policy >' ); - if ( !function_exists( 'wfOutputHandler' ) ) { - function wfOutputHandler( $s ) { - return $s; - } - } - $printer = $main->createPrinterByName( 'php' ); - ob_start( 'wfOutputHandler' ); + ob_start( 'MediaWiki\\OutputHandler::handle' ); $printer->initPrinter(); $printer->execute(); $printer->closePrinter(); @@ -126,7 +120,7 @@ class ApiFormatPhpTest extends ApiFormatTestBase { $config->set( 'MangleFlashPolicy', true ); $printer = $main->createPrinterByName( 'php' ); - ob_start( 'wfOutputHandler' ); + ob_start( 'MediaWiki\\OutputHandler::handle' ); try { $printer->initPrinter(); $printer->execute(); -- 2.20.1