r90194 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90193‎ | r90194 | r90195 >
Date:05:52, 16 June 2011
Author:tstarling
Status:ok (Comments)
Tags:
Comment:
Fixes for r90105, r90193:
* Actually removed $wgProto.
* Per Aryeh's suggestions on the future of $wgServer: made $wgServer detection in DefaultSettings.php more permanent by merging it with the new code from r90105. This means that bug 14977 is properly fixed now.
* Require entry points to set up the autoloader before including DefaultSettings.php. Comments on bug 14977 indicate that at some point in the past, this may have broken something. Anything that breaks now should just be fixed, we need the autoloader. Tested the most common entry points.
* Since the detection code has moved from Installer to WebRequest, I also moved the relevant test file and updated the test. The function under test is now public static, so r90154 is superseded.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.18 (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/WebRequest.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/WebRequestTest.php (added) (history)
  • /trunk/phase3/tests/phpunit/includes/installer (deleted) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.18
——@@ -255,6 +255,8 @@
256256 * (bug 29263) Add LTR class to the shared CSS to be used for left-to-right text
257257 such as SQL queries shown in dberrortext and similar messages in RTL
258258 environments
 259+* (bug 14977) Fixed $wgServer detection in cases where an IPv6 address is used
 260+ as the server name.
259261
260262 === API changes in 1.18 ===
261263 * (bug 26339) Throw warning when truncating an overlarge API result.
Index: trunk/phase3/tests/phpunit/includes/WebRequestTest.php
——@@ -0,0 +1,88 @@
 2+<?php
 3+
 4+class WebRequestTest extends MediaWikiTestCase {
 5+ /**
 6+ * @dataProvider provideDetectServer
 7+ */
 8+ function testDetectServer( $expected, $input, $description ) {
 9+ $oldServer = $_SERVER;
 10+ $_SERVER = $input;
 11+ $result = WebRequest::detectServer();
 12+ $_SERVER = $oldServer;
 13+ $this->assertEquals( $expected, $result, $description );
 14+ }
 15+
 16+ function provideDetectServer() {
 17+ return array(
 18+ array(
 19+ 'http://x',
 20+ array(
 21+ 'HTTP_HOST' => 'x'
 22+ ),
 23+ 'Host header'
 24+ ),
 25+ array(
 26+ 'https://x',
 27+ array(
 28+ 'HTTP_HOST' => 'x',
 29+ 'HTTPS' => 'on',
 30+ ),
 31+ 'Host header with secure'
 32+ ),
 33+ array(
 34+ 'http://x',
 35+ array(
 36+ 'HTTP_HOST' => 'x',
 37+ 'SERVER_PORT' => 80,
 38+ ),
 39+ 'Default SERVER_PORT',
 40+ ),
 41+ array(
 42+ 'http://x',
 43+ array(
 44+ 'HTTP_HOST' => 'x',
 45+ 'HTTPS' => 'off',
 46+ ),
 47+ 'Secure off'
 48+ ),
 49+ array(
 50+ 'http://y',
 51+ array(
 52+ 'SERVER_NAME' => 'y',
 53+ ),
 54+ 'Server name'
 55+ ),
 56+ array(
 57+ 'http://x',
 58+ array(
 59+ 'HTTP_HOST' => 'x',
 60+ 'SERVER_NAME' => 'y',
 61+ ),
 62+ 'Host server name precedence'
 63+ ),
 64+ array(
 65+ 'http://[::1]:81',
 66+ array(
 67+ 'HTTP_HOST' => '[::1]',
 68+ 'SERVER_NAME' => '::1',
 69+ 'SERVER_PORT' => '81',
 70+ ),
 71+ 'Apache bug 26005'
 72+ ),
 73+ array(
 74+ 'http://localhost',
 75+ array(
 76+ 'SERVER_NAME' => '[2001'
 77+ ),
 78+ 'Kind of like lighttpd per commit message in MW r83847',
 79+ ),
 80+ array(
 81+ 'http://[2a01:e35:2eb4:1::2]:777',
 82+ array(
 83+ 'SERVER_NAME' => '[2a01:e35:2eb4:1::2]:777'
 84+ ),
 85+ 'Possible lighttpd environment per bug 14977 comment 13',
 86+ ),
 87+ );
 88+ }
 89+}
Property changes on: trunk/phase3/tests/phpunit/includes/WebRequestTest.php
___________________________________________________________________
Added: svn:eol-style
190 + native
Index: trunk/phase3/includes/installer/Installer.php
——@@ -843,38 +843,7 @@
844844 * Environment check for the server hostname.
845845 */
846846 protected function envCheckServer() {
847 - if ( isset( $_SERVER['HTTPS'] ) && $_SERVER['HTTPS'] == 'on') {
848 - $proto = 'https';
849 - $stdPort = 443;
850 - } else {
851 - $proto = 'http';
852 - $stdPort = 80;
853 - }
854 -
855 - $varNames = array( 'HTTP_HOST', 'SERVER_NAME', 'HOSTNAME', 'SERVER_ADDR' );
856 - $host = 'localhost';
857 - $port = $stdPort;
858 - foreach ( $varNames as $varName ) {
859 - if ( !isset( $_SERVER[$varName] ) ) {
860 - continue;
861 - }
862 - $parts = IP::splitHostAndPort( $_SERVER[$varName] );
863 - if ( !$parts ) {
864 - // Invalid, do not use
865 - continue;
866 - }
867 - $host = $parts[0];
868 - if ( $parts[1] === false ) {
869 - if ( isset( $_SERVER['SERVER_PORT'] ) ) {
870 - $port = $_SERVER['SERVER_PORT'];
871 - } // else leave it as $stdPort
872 - } else {
873 - $port = $parts[1];
874 - }
875 - break;
876 - }
877 -
878 - $server = $proto . '://' . IP::combineHostAndPort( $host, $port, $stdPort );
 847+ $server = WebRequest::detectServer();
879848 $this->showMessage( 'config-using-server', $server );
880849 $this->setVar( 'wgServer', $server );
881850 }
Index: trunk/phase3/includes/WebRequest.php
——@@ -125,6 +125,45 @@
126126 }
127127
128128 /**
 129+ * Work out an appropriate URL prefix containing scheme and host, based on
 130+ * information detected from $_SERVER
 131+ */
 132+ public static function detectServer() {
 133+ if ( isset( $_SERVER['HTTPS'] ) && $_SERVER['HTTPS'] == 'on') {
 134+ $proto = 'https';
 135+ $stdPort = 443;
 136+ } else {
 137+ $proto = 'http';
 138+ $stdPort = 80;
 139+ }
 140+
 141+ $varNames = array( 'HTTP_HOST', 'SERVER_NAME', 'HOSTNAME', 'SERVER_ADDR' );
 142+ $host = 'localhost';
 143+ $port = $stdPort;
 144+ foreach ( $varNames as $varName ) {
 145+ if ( !isset( $_SERVER[$varName] ) ) {
 146+ continue;
 147+ }
 148+ $parts = IP::splitHostAndPort( $_SERVER[$varName] );
 149+ if ( !$parts ) {
 150+ // Invalid, do not use
 151+ continue;
 152+ }
 153+ $host = $parts[0];
 154+ if ( $parts[1] === false ) {
 155+ if ( isset( $_SERVER['SERVER_PORT'] ) ) {
 156+ $port = $_SERVER['SERVER_PORT'];
 157+ } // else leave it as $stdPort
 158+ } else {
 159+ $port = $parts[1];
 160+ }
 161+ break;
 162+ }
 163+
 164+ return $proto . '://' . IP::combineHostAndPort( $host, $port, $stdPort );
 165+ }
 166+
 167+ /**
129168 * Check for title, action, and/or variant data in the URL
130169 * and interpolate it into the GET variables.
131170 * This should only be run after $wgContLang is available,
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -26,10 +26,9 @@
2727 die( 1 );
2828 }
2929
30 -# Create a site configuration object. Not used for much in a default install
31 -if ( !defined( 'MW_COMPILED' ) ) {
32 - require_once( "$IP/includes/SiteConfiguration.php" );
33 -}
 30+# Create a site configuration object. Not used for much in a default install.
 31+# Note: this (and other things) will break if the autoloader is not enabled.
 32+# Please include includes/AutoLoader.php before including this file.
3433 $wgConf = new SiteConfiguration;
3534 /** @endcond */
3635
——@@ -51,37 +50,8 @@
5251 * wrong server, it will redirect incorrectly after you save a page. In that
5352 * case, set this variable to fix it.
5453 */
55 -$wgServer = '';
 54+$wgServer = WebRequest::detectServer();
5655
57 -/** @cond file_level_code */
58 -if( isset( $_SERVER['SERVER_NAME'] )
59 - # KLUGE: lighttpd 1.4.28 truncates IPv6 addresses at the first colon,
60 - # giving bogus hostnames like "[2001"; check for presence of both
61 - # brackets to detect this.
62 - && ($_SERVER['SERVER_NAME'][0] !== '[' || substr($_SERVER['SERVER_NAME'], -1) === ']')
63 - ) {
64 - $serverName = $_SERVER['SERVER_NAME'];
65 -} elseif( isset( $_SERVER['HOSTNAME'] ) ) {
66 - $serverName = $_SERVER['HOSTNAME'];
67 -} elseif( isset( $_SERVER['SERVER_ADDR'] ) ) {
68 - $serverName = $_SERVER['SERVER_ADDR'];
69 -} else {
70 - $serverName = 'localhost';
71 -}
72 -
73 -$wgProto = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') ? 'https' : 'http';
74 -
75 -$wgServer = $wgProto.'://' . $serverName;
76 -# If the port is a non-standard one, add it to the URL
77 -if( isset( $_SERVER['SERVER_PORT'] )
78 - && !strpos( $serverName, ':' )
79 - && ( ( $wgProto == 'http' && $_SERVER['SERVER_PORT'] != 80 )
80 - || ( $wgProto == 'https' && $_SERVER['SERVER_PORT'] != 443 ) ) ) {
81 -
82 - $wgServer .= ":" . $_SERVER['SERVER_PORT'];
83 -}
84 -/** @endcond */
85 -
8656 /************************************************************************//**
8757 * @name Script path settings
8858 * @{
——@@ -986,6 +956,8 @@
987957 * @{
988958 */
989959
 960+$serverName = substr( $wgServer, strrpos( $wgServer, '/' ) + 1 );
 961+
990962 /**
991963 * Site admin email address.
992964 */

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r38214(bug 14977) Installations on servers using IPv6 addresses for $wgServer were ...demon03:23, 30 July 2008
r90105* (bug 28798) Set $wgServer in the default LocalSettings.php...tstarling07:35, 15 June 2011
r90154Followup r90105, ReflectionMethod::setAccessible() requires PHP 5.3.2. Mark i...demon20:57, 15 June 2011
r90193Removed $wgProto. Previously, setting this undocumented global variable to an...tstarling05:13, 16 June 2011

Comments

#Comment by Tim Starling (talk | contribs)   05:58, 16 June 2011

Marking scaptrap due to the remote possibility of fatal errors on Wikimedia due to the need for the autoloader to be loaded before DefaultSettings.php.

Status & tagging log