From 392380ff36e8c6596d0c82dc4d81ed11b6c5188d Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Sat, 1 Oct 2016 22:58:55 -0700 Subject: [PATCH] http: Use Psr\Log instead of wfDebug* MWHttpRequest::factory() will pass in a logger to move the dependency up to the factory instead of individual functions. Change-Id: I4e428f060c90ef49cb3acb3e3dceab64bd952330 --- includes/http/CurlHttpRequest.php | 6 +++--- includes/http/Http.php | 1 + includes/http/MWHttpRequest.php | 33 ++++++++++++++++++++++++++++++- includes/http/PhpHttpRequest.php | 5 ++--- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/includes/http/CurlHttpRequest.php b/includes/http/CurlHttpRequest.php index 7714818658..f58c3a9a5b 100644 --- a/includes/http/CurlHttpRequest.php +++ b/includes/http/CurlHttpRequest.php @@ -108,7 +108,7 @@ class CurlHttpRequest extends MWHttpRequest { if ( $this->followRedirects && $this->canFollowRedirects() ) { MediaWiki\suppressWarnings(); if ( !curl_setopt( $curlHandle, CURLOPT_FOLLOWLOCATION, true ) ) { - wfDebug( __METHOD__ . ": Couldn't set CURLOPT_FOLLOWLOCATION. " . + $this->logger->debug( __METHOD__ . ": Couldn't set CURLOPT_FOLLOWLOCATION. " . "Probably open_basedir is set.\n" ); // Continue the processing. If it were in curl_setopt_array, // processing would have halted on its entry @@ -149,13 +149,13 @@ class CurlHttpRequest extends MWHttpRequest { public function canFollowRedirects() { $curlVersionInfo = curl_version(); if ( $curlVersionInfo['version_number'] < 0x071304 ) { - wfDebug( "Cannot follow redirects with libcurl < 7.19.4 due to CVE-2009-0037\n" ); + $this->logger->debug( "Cannot follow redirects with libcurl < 7.19.4 due to CVE-2009-0037\n" ); return false; } if ( version_compare( PHP_VERSION, '5.6.0', '<' ) ) { if ( strval( ini_get( 'open_basedir' ) ) !== '' ) { - wfDebug( "Cannot follow redirects when open_basedir is set\n" ); + $this->logger->debug( "Cannot follow redirects when open_basedir is set\n" ); return false; } } diff --git a/includes/http/Http.php b/includes/http/Http.php index 46d2047dc5..43ae2d0e8f 100644 --- a/includes/http/Http.php +++ b/includes/http/Http.php @@ -50,6 +50,7 @@ class Http { * to avoid attacks on intranet services accessible by HTTP. * - userAgent A user agent, if you want to override the default * MediaWiki/$wgVersion + * - logger A \Psr\Logger\LoggerInterface instance for debug logging * @param string $caller The method making this request, for profiling * @return string|bool (bool)false on failure or a string on success */ diff --git a/includes/http/MWHttpRequest.php b/includes/http/MWHttpRequest.php index 30a5c21b2c..458854ab2d 100644 --- a/includes/http/MWHttpRequest.php +++ b/includes/http/MWHttpRequest.php @@ -18,6 +18,11 @@ * @file */ +use MediaWiki\Logger\LoggerFactory; +use Psr\Log\LoggerInterface; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\NullLogger; + /** * This wrapper class will call out to curl (if available) or fallback * to regular PHP if necessary for handling internal HTTP requests. @@ -25,7 +30,7 @@ * Renamed from HttpRequest to MWHttpRequest to avoid conflict with * PHP's HTTP extension. */ -class MWHttpRequest { +class MWHttpRequest implements LoggerAwareInterface { const SUPPORTS_FILE_POSTS = false; protected $content; @@ -67,6 +72,11 @@ class MWHttpRequest { */ protected $profileName; + /** + * @var LoggerInterface; + */ + protected $logger; + /** * @param string $url Url to use. If protocol-relative, will be expanded to an http:// URL * @param array $options (optional) extra params to pass (see Http::request()) @@ -81,6 +91,12 @@ class MWHttpRequest { $this->url = wfExpandUrl( $url, PROTO_HTTP ); $this->parsedUrl = wfParseUrl( $this->url ); + if ( isset( $options['logger'] ) ) { + $this->logger = $options['logger']; + } else { + $this->logger = new NullLogger(); + } + if ( !$this->parsedUrl || !Http::isValidURI( $this->url ) ) { $this->status = Status::newFatal( 'http-invalid-url', $url ); } else { @@ -124,6 +140,13 @@ class MWHttpRequest { $this->profileName = $caller; } + /** + * @param LoggerInterface $logger + */ + public function setLogger( LoggerInterface $logger ) { + $this->logger = $logger; + } + /** * Simple function to test if we can make any sort of requests at all, using * cURL or fopen() @@ -150,6 +173,14 @@ class MWHttpRequest { ' Http::$httpEngine is set to "curl"' ); } + if ( !is_array( $options ) ) { + $options = []; + } + + if ( !isset( $options['logger'] ) ) { + $options['logger'] = LoggerFactory::getInstance( 'http' ); + } + switch ( Http::$httpEngine ) { case 'curl': return new CurlHttpRequest( $url, $options, $caller, Profiler::instance() ); diff --git a/includes/http/PhpHttpRequest.php b/includes/http/PhpHttpRequest.php index f422ff780a..2af000fac0 100644 --- a/includes/http/PhpHttpRequest.php +++ b/includes/http/PhpHttpRequest.php @@ -17,7 +17,6 @@ * * @file */ -use MediaWiki\Logger\LoggerFactory; class PhpHttpRequest extends MWHttpRequest { @@ -212,7 +211,7 @@ class PhpHttpRequest extends MWHttpRequest { $url = $this->getResponseHeader( "Location" ); if ( !Http::isValidURI( $url ) ) { - wfDebug( __METHOD__ . ": insecure redirection\n" ); + $this->logger->debug( __METHOD__ . ": insecure redirection\n" ); break; } } while ( true ); @@ -224,7 +223,7 @@ class PhpHttpRequest extends MWHttpRequest { if ( $fh === false ) { if ( $this->fopenErrors ) { - LoggerFactory::getInstance( 'http' )->warning( __CLASS__ + $this->logger->warning( __CLASS__ . ': error opening connection: {errstr1}', $this->fopenErrors ); } $this->status->fatal( 'http-request-error' ); -- 2.20.1