(bug NNNNN) Rewrite most of wfExpandUrl() to handle protocol-relative URLs properly...

* Fix a bug in rNNNNN where URLs like '/wiki/Foo' weren't expanded completely if $wgServer was protocol-relative. This caused bug NNNNN.
* Add an optional second parameter to wfExpandUrl(), which takes one the PROT_* constants. This allows the caller to determine which protocol should be used if the given URL is protocol-relative, or the given URL is domain-relative but $wgServer is protocol-relative. The options are PROT_HTTP (use http), PROT_HTTPS (use https), PROT_RELATIVE (keep the URL as protocol-relative), and PROT_CURRENT (use http if the current request is http, or https if the current request is https; this is the default).
* Factor the protocol/port detection part of WebRequest::detectServer() out into detectProtocolAndStdPort(), and add detectProtocol() as a wrapper. The latter is used by wfExpandUrl() in PROT_CURRENT mode.
* Rewrite the test suite to test all possible combinations of $wgServer, $defaultProto, $url and HTTP/HTTPS mode. This means the test suite now has 120 test cases rather than 4.

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index d6c433e..3c893b0 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -428,24 +428,44 @@ function wfAppendQuery( $url, $query ) {
        return $url;
 }
 
+define( 'PROT_HTTP', 'http://' );
+define( 'PROT_HTTPS', 'https://' );
+define( 'PROT_RELATIVE', '//' );
+define( 'PROT_CURRENT', null );
+
 /**
  * Expand a potentially local URL to a fully-qualified URL.  Assumes $wgServer
  * is correct.
+ * 
+ * The meaning of the PROT_* constants is as follows:
+ * PROT_HTTP: Output a URL starting with http://
+ * PROT_HTTPS: Output a URL starting with https://
+ * PROT_RELATIVE: Output a URL starting with // (protocol-relative URL)
+ * PROT_CURRENT: Output a URL starting with either http:// or https:// , depending on which protocol was used for the current incoming request
  *
  * @todo this won't work with current-path-relative URLs
  * like "subdir/foo.html", etc.
  *
  * @param $url String: either fully-qualified or a local path + query
+ * @param $defaultProto Mixed: one of the PROT_* constants. Determines the protocol to use if $url or $wgServer is protocol-relative
  * @return string Fully-qualified URL
  */
-function wfExpandUrl( $url ) {
+function wfExpandUrl( $url, $defaultProto = PROT_CURRENT ) {
        global $wgServer;
+       if ( $defaultProto === PROT_CURRENT ) {
+               $defaultProto = WebRequest::detectProtocol() . '://';
+       }
+       
+       // Analyze $wgServer to obtain its protocol
+       $bits = wfParseUrl( $wgServer );
+       $serverHasProto = $bits && $bits['scheme'] != '';
+       $defaultProtoWithoutSlashes = substr( $defaultProto, 0, -2 );
+       
        if( substr( $url, 0, 2 ) == '//' ) {
-               $bits = wfParseUrl( $wgServer );
-               $scheme = $bits && $bits['scheme'] !== '' ? $bits['scheme'] : 'http';
-               return $scheme . ':' . $url;
+               return $defaultProtoWithoutSlashes . $url;
        } elseif( substr( $url, 0, 1 ) == '/' ) {
-               return $wgServer . $url;
+               // If $wgServer is protocol-relative, prepend $defaultProtoWithoutSlashes, otherwise leave it alone
+               return ( $serverHasProto ? '' : $defaultProtoWithoutSlashes ) . $wgServer . $url;
        } else {
                return $url;
        }

diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 196962c..fd21c9a 100644 (file)
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -131,13 +131,7 @@ class WebRequest {
         * @return string
         */
        public static function detectServer() {
-               if ( isset( $_SERVER['HTTPS'] ) && $_SERVER['HTTPS'] == 'on') {
-                       $proto = 'https';
-                       $stdPort = 443;
-               } else {
-                       $proto = 'http';
-                       $stdPort = 80;
-               }
+               list( $proto, $stdPort ) = self::detectProtocolAndStdPort();
 
                $varNames = array( 'HTTP_HOST', 'SERVER_NAME', 'HOSTNAME', 'SERVER_ADDR' );
                $host = 'localhost';
@@ -164,6 +158,15 @@ class WebRequest {
 
                return $proto . '://' . IP::combineHostAndPort( $host, $port, $stdPort );
        }
+       
+       public static function detectProtocolAndStdPort() {
+               return ( isset( $_SERVER['HTTPS'] ) && $_SERVER['HTTPS'] == 'on' ) ? array( 'https', 443 ) : array( 'http', 80 );
+       }
+       
+       public static function detectProtocol() {
+               list( $proto, $stdPort ) = self::detectProtocolAndStdPort();
+               return $proto;
+       }
 
        /**
         * Check for title, action, and/or variant data in the URL

diff --git a/tests/phpunit/includes/GlobalFunctions/wfExpandUrl.php b/tests/phpunit/includes/GlobalFunctions/wfExpandUrl.php
index 624ee8c..fe60bdc 100644 (file)
--- a/tests/phpunit/includes/GlobalFunctions/wfExpandUrl.php
+++ b/tests/phpunit/includes/GlobalFunctions/wfExpandUrl.php
@@ -5,8 +5,23 @@
 
 class wfExpandUrl extends MediaWikiTestCase {
        /** @dataProvider provideExpandableUrls */
-       public function testWfExpandUrl( $fullUrl, $shortUrl, $message ) {
-               $this->assertEquals( $fullUrl, wfExpandUrl( $shortUrl ), $message );
+       public function testWfExpandUrl( $fullUrl, $shortUrl, $defaultProto, $server, $httpsMode, $message ) {
+               // Fake $wgServer
+               global $wgServer;
+               $oldServer = $wgServer;
+               $wgServer = $server;
+               
+               // Fake $_SERVER['HTTPS'] if needed
+               if ( $httpsMode ) {
+                       $_SERVER['HTTPS'] = 'on';
+               } else {
+                       unset( $_SERVER['HTTPS'] );
+               }
+               
+               $this->assertEquals( $fullUrl, wfExpandUrl( $shortUrl, $defaultProto ), $message );
+               
+               // Restore $wgServer
+               $wgServer = $oldServer;
        }
 
        /**
@@ -15,13 +30,37 @@ class wfExpandUrl extends MediaWikiTestCase {
         * @return array
         */
        public function provideExpandableUrls() {
-               global $wgServer;
-               return array(
-                       array( "$wgServer/wiki/FooBar", '/wiki/FooBar', 'Testing expanding URL beginning with /' ),
-                       array( 'http://example.com', 'http://example.com', 'Testing fully qualified http URLs (no need to expand)' ),
-                       array( 'https://example.com', 'https://example.com', 'Testing fully qualified https URLs (no need to expand)' ),
-                       # Would be nice to support this, see fixme on wfExpandUrl()
-                       array( "wiki/FooBar", 'wiki/FooBar', "Test non-expandable relative URLs" ),
-               );
+               $modes = array( 'http', 'https' );
+               $servers = array( 'http' => 'http://example.com', 'https' => 'https://example.com', 'protocol-relative' => '//example.com' );
+               $defaultProtos = array( 'http' => PROT_HTTP, 'https' => PROT_HTTPS, 'protocol-relative' => PROT_RELATIVE, 'current' => PROT_CURRENT );
+               
+               $retval = array();
+               foreach ( $modes as $mode ) {
+                       $httpsMode = $mode == 'https';
+                       foreach ( $servers as $serverDesc => $server ) {
+                               foreach ( $defaultProtos as $protoDesc => $defaultProto ) {
+                                       $retval[] = array( 'http://example.com', 'http://example.com', $defaultProto, $server, $httpsMode, "Testing fully qualified http URLs (no need to expand) (defaultProto: $protoDesc , wgServer: $server, current request protocol: $mode )" );
+                                       $retval[] = array( 'https://example.com', 'https://example.com', $defaultProto, $server, $httpsMode, "Testing fully qualified https URLs (no need to expand) (defaultProto: $protoDesc , wgServer: $server, current request protocol: $mode )" );
+                                       # Would be nice to support this, see fixme on wfExpandUrl()
+                                       $retval[] = array( "wiki/FooBar", 'wiki/FooBar', $defaultProto, $server, $httpsMode, "Test non-expandable relative URLs (defaultProto: $protoDesc , wgServer: $server, current request protocol: $mode )" );
+                                       
+                                       // Determine expected protocol
+                                       $p = $protoDesc . ':'; // default case
+                                       if ( $protoDesc == 'protocol-relative' ) {
+                                               $p = '';
+                                       } else if ( $protoDesc == 'current' ) {
+                                               $p = "$mode:";
+                                       } else {
+                                               $p = $protoDesc . ':';
+                                       }
+                                       // Determine expected server name
+                                       $srv = $serverDesc == 'protocol-relative' ? $p . $server : $server;
+                                       
+                                       $retval[] = array( "$p//wikipedia.org", '//wikipedia.org', $defaultProto, $server, $httpsMode, "Test protocol-relative URL (defaultProto: $protoDesc, wgServer: $server, current request protocol: $mode )" );
+                                       $retval[] = array( "$srv/wiki/FooBar", '/wiki/FooBar', $defaultProto, $server, $httpsMode, "Testing expanding URL beginning with / (defaultProto: $protoDesc , wgServer: $server, current request protocol: $mode )" );
+                               }
+                       }
+               }
+               return $retval;
        }
 }