$wgHttpsPort should only be used in very special cases

When expanding a URL, don't overwrite an explicitly specified port or
add a port to a foreign URL. $wgHttpsPort is only useful for a very
specific case: when $wgServer is protocol-relative and HTTPS is
requested.

Documented correct use of $wgHttpsPort in DefaultSettings.php. Fixed
invalid "@see", in Doxygen it can only point to "classes, functions,
methods, variables, files or URL".

Added test cases which previously failed.

Change-Id: Id65c58300d22712212b6605711ff916916e8768b

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 2642884..4962398 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -8606,9 +8606,14 @@ $wgSiteTypes = [
 $wgPagePropsHaveSortkey = true;
 
 /**
- * Port where you have HTTPS running
- * Supports HTTPS on non-standard ports
- * @see T67184
+ * For installations where the canonical server is HTTP but HTTPS is optionally
+ * supported, you can specify a non-standard HTTPS port here. $wgServer should
+ * be a protocol-relative URL.
+ *
+ * If HTTPS is always used, just specify the port number in $wgServer.
+ *
+ * @see https://phabricator.wikimedia.org/T67184
+ *
  * @since 1.24
  */
 $wgHttpsPort = 443;

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 543978b..18bacc5 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -556,17 +556,24 @@ function wfExpandUrl( $url, $defaultProto = PROTO_CURRENT ) {
        } elseif ( substr( $url, 0, 1 ) == '/' ) {
                // If $serverUrl is protocol-relative, prepend $defaultProtoWithoutSlashes,
                // otherwise leave it alone.
-               $url = ( $serverHasProto ? '' : $defaultProtoWithoutSlashes ) . $serverUrl . $url;
+               if ( $serverHasProto ) {
+                       $url = $serverUrl . $url;
+               } else {
+                       // If an HTTPS URL is synthesized from a protocol-relative $wgServer, allow the
+                       // user to override the port number (T67184)
+                       if ( $defaultProto === PROTO_HTTPS && $wgHttpsPort != 443 ) {
+                               if ( isset( $bits['port'] ) ) {
+                                       throw new Exception( 'A protocol-relative $wgServer may not contain a port number' );
+                               }
+                               $url = $defaultProtoWithoutSlashes . $serverUrl . ':' . $wgHttpsPort . $url;
+                       } else {
+                               $url = $defaultProtoWithoutSlashes . $serverUrl . $url;
+                       }
+               }
        }
 
        $bits = wfParseUrl( $url );
 
-       // ensure proper port for HTTPS arrives in URL
-       // https://phabricator.wikimedia.org/T67184
-       if ( $defaultProto === PROTO_HTTPS && $wgHttpsPort != 443 ) {
-               $bits['port'] = $wgHttpsPort;
-       }
-
        if ( $bits && isset( $bits['path'] ) ) {
                $bits['path'] = wfRemoveDotSegments( $bits['path'] );
                return wfAssembleUrl( $bits );

diff --git a/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php b/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php
index 1cd320f..159e4ad 100644 (file)
--- a/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php
+++ b/tests/phpunit/includes/GlobalFunctions/wfExpandUrlTest.php
@@ -8,13 +8,14 @@ class WfExpandUrlTest extends MediaWikiTestCase {
         * @dataProvider provideExpandableUrls
         */
        public function testWfExpandUrl( $fullUrl, $shortUrl, $defaultProto,
-               $server, $canServer, $httpsMode, $message
+               $server, $canServer, $httpsMode, $httpsPort, $message
        ) {
                // Fake $wgServer, $wgCanonicalServer and $wgRequest->getProtocol()
                $this->setMwGlobals( [
                        'wgServer' => $server,
                        'wgCanonicalServer' => $canServer,
-                       'wgRequest' => new FauxRequest( [], false, null, $httpsMode ? 'https' : 'http' )
+                       'wgRequest' => new FauxRequest( [], false, null, $httpsMode ? 'https' : 'http' ),
+                       'wgHttpsPort' => $httpsPort
                ] );
 
                $this->assertEquals( $fullUrl, wfExpandUrl( $shortUrl, $defaultProto ), $message );
@@ -49,14 +50,14 @@ class WfExpandUrlTest extends MediaWikiTestCase {
                                        foreach ( $defaultProtos as $protoDesc => $defaultProto ) {
                                                $retval[] = [
                                                        'http://example.com', 'http://example.com',
-                                                       $defaultProto, $server, $canServer, $httpsMode,
+                                                       $defaultProto, $server, $canServer, $httpsMode, 443,
                                                        "Testing fully qualified http URLs (no need to expand) "
                                                                . "(defaultProto: $protoDesc , wgServer: $server, "
                                                                . "wgCanonicalServer: $canServer, current request protocol: $mode )"
                                                ];
                                                $retval[] = [
                                                        'https://example.com', 'https://example.com',
-                                                       $defaultProto, $server, $canServer, $httpsMode,
+                                                       $defaultProto, $server, $canServer, $httpsMode, 443,
                                                        "Testing fully qualified https URLs (no need to expand) "
                                                                . "(defaultProto: $protoDesc , wgServer: $server, "
                                                                . "wgCanonicalServer: $canServer, current request protocol: $mode )"
@@ -64,7 +65,7 @@ class WfExpandUrlTest extends MediaWikiTestCase {
                                                # Would be nice to support this, see fixme on wfExpandUrl()
                                                $retval[] = [
                                                        "wiki/FooBar", 'wiki/FooBar',
-                                                       $defaultProto, $server, $canServer, $httpsMode,
+                                                       $defaultProto, $server, $canServer, $httpsMode, 443,
                                                        "Test non-expandable relative URLs (defaultProto: $protoDesc, "
                                                                . "wgServer: $server, wgCanonicalServer: $canServer, "
                                                                . "current request protocol: $mode )"
@@ -91,7 +92,7 @@ class WfExpandUrlTest extends MediaWikiTestCase {
 
                                                $retval[] = [
                                                        "$p//wikipedia.org", '//wikipedia.org',
-                                                       $defaultProto, $server, $canServer, $httpsMode,
+                                                       $defaultProto, $server, $canServer, $httpsMode, 443,
                                                        "Test protocol-relative URL (defaultProto: $protoDesc, "
                                                                . "wgServer: $server, wgCanonicalServer: $canServer, "
                                                                . "current request protocol: $mode )"
@@ -103,6 +104,7 @@ class WfExpandUrlTest extends MediaWikiTestCase {
                                                        $server,
                                                        $canServer,
                                                        $httpsMode,
+                                                       443,
                                                        "Testing expanding URL beginning with / (defaultProto: $protoDesc, "
                                                                . "wgServer: $server, wgCanonicalServer: $canServer, "
                                                                . "current request protocol: $mode )"
@@ -112,6 +114,39 @@ class WfExpandUrlTest extends MediaWikiTestCase {
                        }
                }
 
+               // Don't add HTTPS port to foreign URLs
+               $retval[] = [
+                       'https://foreign.example.com/foo',
+                       'https://foreign.example.com/foo',
+                       PROTO_HTTPS,
+                       '//wiki.example.com',
+                       'http://wiki.example.com',
+                       'https',
+                       111,
+                       "Don't add HTTPS port to foreign URLs"
+               ];
+               $retval[] = [
+                       'https://foreign.example.com:222/foo',
+                       'https://foreign.example.com:222/foo',
+                       PROTO_HTTPS,
+                       '//wiki.example.com',
+                       'http://wiki.example.com',
+                       'https',
+                       111,
+                       "Don't overwrite HTTPS port of foreign URLs"
+               ];
+               // Do add HTTPS port to local URLs
+               $retval[] = [
+                       'https://wiki.example.com:111/foo',
+                       '/foo',
+                       PROTO_HTTPS,
+                       '//wiki.example.com',
+                       'http://wiki.example.com',
+                       'https',
+                       111,
+                       "Do add HTTPS port to protocol-relative URLs"
+               ];
+
                return $retval;
        }
 }