Introduce WebRequest::getProtocol()

The use of static server detection outside of its intended use case
(i.e. at the start of DefaultSettings.php), for example in r93258, was
an architectural error. Every other bit of information about the web
request in non-setup code comes from non-static methods of WebRequest,
which allows the request object to be meaningfully replaced or
subclassed. The situation became increasingly ridiculous as more
callers of WebRequest::detectProtocol() were introduced. Two of the
callers were calling it non-statically! I suppose they had the right
idea, in a way.

Using a non-static call allows caching, which is a nice additional
benefit.

WebRequest::detectProtocolAndStdPort() was introduced in r93258 as part of
the introduction of WebRequest::detectProtocol(). It was basically
useless. Grep indicates there was only one caller in core and WMF
deployed extensions, and it is patched here.

Change-Id: Ia0a61e98fbff7a46ceaeebcb02236e5eac3df0e1

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index ef0ec2c..5900d07 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -495,7 +495,7 @@ function wfAppendQuery( $url, $query ) {
  *    no valid URL can be constructed
  */
 function wfExpandUrl( $url, $defaultProto = PROTO_CURRENT ) {
-       global $wgServer, $wgCanonicalServer, $wgInternalServer;
+       global $wgServer, $wgCanonicalServer, $wgInternalServer, $wgRequest;
        $serverUrl = $wgServer;
        if ( $defaultProto === PROTO_CANONICAL ) {
                $serverUrl = $wgCanonicalServer;
@@ -505,7 +505,7 @@ function wfExpandUrl( $url, $defaultProto = PROTO_CURRENT ) {
                $serverUrl = $wgInternalServer;
        }
        if ( $defaultProto === PROTO_CURRENT ) {
-               $defaultProto = WebRequest::detectProtocol() . '://';
+               $defaultProto = $wgRequest->getProtocol() . '://';
        }
 
        // Analyze $serverUrl to obtain its protocol

diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index b17cb9e..462b312 100644 (file)
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -50,6 +50,12 @@ class WebRequest {
         */
        private $ip;
 
+       /**
+        * Cached URL protocol
+        * @var string
+        */
+       private $protocol;
+
        public function __construct() {
                /// @todo FIXME: This preemptive de-quoting can interfere with other web libraries
                ///        and increases our memory footprint. It would be cleaner to do on
@@ -160,7 +166,8 @@ class WebRequest {
         * @return string
         */
        public static function detectServer() {
-               list( $proto, $stdPort ) = self::detectProtocolAndStdPort();
+               $proto = self::detectProtocol();
+               $stdPort = $proto === 'https' ? 443 : 80;
 
                $varNames = array( 'HTTP_HOST', 'SERVER_NAME', 'HOSTNAME', 'SERVER_ADDR' );
                $host = 'localhost';
@@ -189,25 +196,32 @@ class WebRequest {
        }
 
        /**
+        * Detect the protocol from $_SERVER.
+        * This is for use prior to Setup.php, when no WebRequest object is available.
+        * At other times, use the non-static function getProtocol().
+        *
         * @return array
         */
-       public static function detectProtocolAndStdPort() {
+       public static function detectProtocol() {
                if ( ( isset( $_SERVER['HTTPS'] ) && $_SERVER['HTTPS'] == 'on' ) ||
                        ( isset( $_SERVER['HTTP_X_FORWARDED_PROTO'] ) &&
                        $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https' ) ) {
-                       $arr = array( 'https', 443 );
+                       return 'https';
                } else {
-                       $arr = array( 'http', 80 );
+                       return 'http';
                }
                return $arr;
        }
 
        /**
+        * Get the current URL protocol (http or https)
         * @return string
         */
-       public static function detectProtocol() {
-               list( $proto, ) = self::detectProtocolAndStdPort();
-               return $proto;
+       public function getProtocol() {
+               if ( $this->protocol === null ) {
+                       $this->protocol = self::detectProtocol();
+               }
+               return $this->protocol;
        }
 
        /**
@@ -1312,9 +1326,10 @@ class FauxRequest extends WebRequest {
         *   fake GET/POST values
         * @param bool $wasPosted whether to treat the data as POST
         * @param $session Mixed: session array or null
+        * @param string $protocol 'http' or 'https'
         * @throws MWException
         */
-       public function __construct( $data = array(), $wasPosted = false, $session = null ) {
+       public function __construct( $data = array(), $wasPosted = false, $session = null, $protocol = 'http' ) {
                if ( is_array( $data ) ) {
                        $this->data = $data;
                } else {
@@ -1324,6 +1339,7 @@ class FauxRequest extends WebRequest {
                if ( $session ) {
                        $this->session = $session;
                }
+               $this->protocol = $protocol;
        }
 
        /**
@@ -1385,6 +1401,10 @@ class FauxRequest extends WebRequest {
                $this->notImplemented( __METHOD__ );
        }
 
+       public function getProtocol() {
+               return $this->protocol;
+       }
+
        /**
         * @param string $name The name of the header to get (case insensitive).
         * @return bool|string
@@ -1524,4 +1544,8 @@ class DerivativeRequest extends FauxRequest {
        public function getIP() {
                return $this->base->getIP();
        }
+
+       public function getProtocol() {
+               return $this->base->getProtocol();
+       }
 }

diff --git a/includes/Wiki.php b/includes/Wiki.php
index a690176..50bba7b 100644 (file)
--- a/includes/Wiki.php
+++ b/includes/Wiki.php
@@ -540,7 +540,7 @@ class MediaWiki {
                                        && $this->context->getUser()->requiresHTTPS()
                                )
                        ) &&
-                       $request->detectProtocol() == 'http'
+                       $request->getProtocol() == 'http'
                ) {
                        $oldUrl = $request->getFullRequestURL();
                        $redirUrl = str_replace( 'http://', 'https://', $oldUrl );

diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php
index 5ac3e65..f40966c 100644 (file)
--- a/includes/specials/SpecialUserlogin.php
+++ b/includes/specials/SpecialUserlogin.php
@@ -106,7 +106,7 @@ class LoginForm extends SpecialPage {
                $this->mAction = $request->getVal( 'action' );
                $this->mRemember = $request->getCheck( 'wpRemember' );
                $this->mFromHTTP = $request->getBool( 'fromhttp', false );
-               $this->mStickHTTPS = ( !$this->mFromHTTP && $request->detectProtocol() === 'https' ) || $request->getBool( 'wpForceHttps', false );
+               $this->mStickHTTPS = ( !$this->mFromHTTP && $request->getProtocol() === 'https' ) || $request->getBool( 'wpForceHttps', false );
                $this->mLanguage = $request->getText( 'uselang' );
                $this->mSkipCookieCheck = $request->getCheck( 'wpSkipCookieCheck' );
                $this->mToken = ( $this->mType == 'signup' ) ? $request->getVal( 'wpCreateaccountToken' ) : $request->getVal( 'wpLoginToken' );
@@ -168,7 +168,7 @@ class LoginForm extends SpecialPage {
 
                // If logging in and not on HTTPS, either redirect to it or offer a link.
                global $wgSecureLogin;
-               if ( WebRequest::detectProtocol() !== 'https' ) {
+               if ( $this->mRequest->getProtocol() !== 'https' ) {
                        $title = $this->getFullTitle();
                        $query = array(
                                'returnto' => $this->mReturnTo,
@@ -1196,7 +1196,7 @@ class LoginForm extends SpecialPage {
                $template->set( 'secureLoginUrl', $this->mSecureLoginUrl );
                // Use loginend-https for HTTPS requests if it's not blank, loginend otherwise
                // Ditto for signupend.  New forms use neither.
-               $usingHTTPS = WebRequest::detectProtocol() == 'https';
+               $usingHTTPS = $this->mRequest->getProtocol() == 'https';
                $loginendHTTPS = $this->msg( 'loginend-https' );
                $signupendHTTPS = $this->msg( 'signupend-https' );
                if ( $usingHTTPS && !$loginendHTTPS->isBlank() ) {

diff --git a/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php b/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php
index 41230a1..b06f3d2 100644 (file)
--- a/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php
+++ b/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php
@@ -7,19 +7,13 @@ class WfExpandUrlTest extends MediaWikiTestCase {
         * @dataProvider provideExpandableUrls
         */
        public function testWfExpandUrl( $fullUrl, $shortUrl, $defaultProto, $server, $canServer, $httpsMode, $message ) {
-               // Fake $wgServer and $wgCanonicalServer
+               // Fake $wgServer, $wgCanonicalServer and $wgRequest->getProtocol()
                $this->setMwGlobals( array(
                        'wgServer' => $server,
                        'wgCanonicalServer' => $canServer,
+                       'wgRequest' => new FauxRequest( array(), false, null, $httpsMode ? 'https' : 'http' )
                ) );
 
-               // Fake $_SERVER['HTTPS'] if needed
-               if ( $httpsMode ) {
-                       $_SERVER['HTTPS'] = 'on';
-               } else {
-                       unset( $_SERVER['HTTPS'] );
-               }
-
                $this->assertEquals( $fullUrl, wfExpandUrl( $shortUrl, $defaultProto ), $message );
        }