From 89629154496932504bebbcb06d85e94cb64b0a0d Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Tue, 10 Jul 2018 10:05:07 +1000 Subject: [PATCH] $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 --- includes/DefaultSettings.php | 11 +++-- includes/GlobalFunctions.php | 21 ++++++--- .../GlobalFunctions/wfExpandUrlTest.php | 47 ++++++++++++++++--- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 26428842f9..4962398a94 100644 --- 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 543978b56e..18bacc5c9d 100644 --- 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 1cd320fa00..159e4ad9a4 100644 --- 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; } } -- 2.20.1