r73950 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r73949‎ | r73950 | r73951 >
Date:15:47, 29 September 2010
Author:platonides
Status:resolved (Comments)
Tags:
Comment:
Remove $wgServerName. Its only usage was for {{servername}}, and needed to be kept in sync with $wgServer in LocalSettings.
None of the 3 globals based on it changed if you set it in LocalSettings.

Note that all those !isset( $wgServerName ) in ApiTests were useless, since if not in LocalSettings it would be 'localhost', not null (as still are those !isset( $wgServer )).
Modified paths:
  • /trunk/phase3/docs/distributors.txt (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/maintenance/tests/parser/parserTest.inc (modified) (history)
  • /trunk/phase3/maintenance/tests/parser/parserTests.txt (modified) (history)
  • /trunk/phase3/maintenance/tests/phpunit/includes/api/ApiTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/phpunit/includes/api/ApiTest.php
——@@ -54,11 +54,10 @@
5555 }
5656
5757 function testApi() {
58 - global $wgServerName, $wgServer;
 58+ global $wgServer;
5959
60 - if ( !isset( $wgServerName ) || !isset( $wgServer ) ) {
61 - $this->markTestIncomplete( 'This test needs $wgServerName and $wgServer to ' .
62 - 'be set in LocalSettings.php' );
 60+ if ( !isset( $wgServer ) ) {
 61+ $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' );
6362 }
6463 /* Haven't thought about test ordering yet -- but this depends on HttpTest.php */
6564 $resp = Http::get( self::$apiUrl . "?format=xml" );
——@@ -70,11 +69,10 @@
7170 }
7271
7372 function testApiLoginNoName() {
74 - global $wgServerName, $wgServer;
 73+ global $wgServer;
7574
76 - if ( !isset( $wgServerName ) || !isset( $wgServer ) ) {
77 - $this->markTestIncomplete( 'This test needs $wgServerName and $wgServer to ' .
78 - 'be set in LocalSettings.php' );
 75+ if ( !isset( $wgServer ) ) {
 76+ $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' );
7977 }
8078 $resp = Http::post( self::$apiUrl . "?action=login&format=xml",
8179 array( "postData" => array(
——@@ -89,11 +87,10 @@
9088 }
9189
9290 function testApiLoginBadPass() {
93 - global $wgServerName, $wgServer;
 91+ global $wgServer;
9492
95 - if ( !isset( $wgServerName ) || !isset( $wgServer ) ) {
96 - $this->markTestIncomplete( 'This test needs $wgServerName and $wgServer to ' .
97 - 'be set in LocalSettings.php' );
 93+ if ( !isset( $wgServer ) ) {
 94+ $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' );
9895 }
9996 $resp = Http::post( self::$apiUrl . "?action=login&format=xml",
10097 array( "postData" => array(
——@@ -124,11 +121,10 @@
125122 }
126123
127124 function testApiLoginGoodPass() {
128 - global $wgServerName, $wgServer;
 125+ global $wgServer;
129126
130 - if ( !isset( $wgServerName ) || !isset( $wgServer ) ) {
131 - $this->markTestIncomplete( 'This test needs $wgServerName and $wgServer to ' .
132 - 'be set in LocalSettings.php' );
 127+ if ( !isset( $wgServer ) ) {
 128+ $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' );
133129 }
134130 $req = HttpRequest::factory( self::$apiUrl . "?action=login&format=xml",
135131 array( "method" => "POST",
——@@ -163,11 +159,10 @@
164160 }
165161
166162 function testApiGotCookie() {
167 - global $wgServerName, $wgServer, $wgScriptPath;
 163+ global $wgServer, $wgScriptPath;
168164
169 - if ( !isset( $wgServerName ) || !isset( $wgServer ) ) {
170 - $this->markTestIncomplete( 'This test needs $wgServerName and $wgServer to ' .
171 - 'be set in LocalSettings.php' );
 165+ if ( !isset( $wgServer ) ) {
 166+ $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' );
172167 }
173168 $req = HttpRequest::factory( self::$apiUrl . "?action=login&format=xml",
174169 array( "method" => "POST",
——@@ -193,7 +188,9 @@
194189 $req->execute();
195190
196191 $cj = $req->getCookieJar();
197 - $serializedCookie = $cj->serializeToHttpRequest( $wgScriptPath, $wgServerName );
 192+ $serverName = parse_url( $wgServer, PHP_URL_HOST );
 193+ $this->assertNotEquals( false, $serverName );
 194+ $serializedCookie = $cj->serializeToHttpRequest( $wgScriptPath, $serverName );
198195 $this->assertNotEquals( '', $serializedCookie );
199196 $this->assertRegexp( '/_session=[^;]*; .*UserID=[0-9]*; .*UserName=' . self::$userName . '; .*Token=/', $serializedCookie );
200197
——@@ -206,11 +203,10 @@
207204 */
208205 function testApiListPages( CookieJar $cj ) {
209206 $this->markTestIncomplete( "Not done with this yet" );
210 - global $wgServerName, $wgServer;
 207+ global $wgServer;
211208
212 - if ( $wgServerName == "localhost" || $wgServer == "http://localhost" ) {
213 - $this->markTestIncomplete( 'This test needs $wgServerName and $wgServer to ' .
214 - 'be set in LocalSettings.php' );
 209+ if ( $wgServer == "http://localhost" ) {
 210+ $this->markTestIncomplete( 'This test needs $wgServer to be set in LocalSettings.php' );
215211 }
216212 $req = HttpRequest::factory( self::$apiUrl . "?action=query&format=xml&prop=revisions&" .
217213 "titles=Main%20Page&rvprop=timestamp|user|comment|content" );
Index: trunk/phase3/maintenance/tests/parser/parserTest.inc
——@@ -532,7 +532,7 @@
533533 self::getOptionValue( 'wgLinkHolderBatchSize', $opts, 1000 );
534534
535535 $settings = array(
536 - 'wgServer' => 'http://localhost',
 536+ 'wgServer' => 'http://Britney-Spears',
537537 'wgScript' => '/index.php',
538538 'wgScriptPath' => '/',
539539 'wgArticlePath' => '/wiki/$1',
——@@ -549,7 +549,6 @@
550550 'wgStylePath' => '/skins',
551551 'wgStyleSheetPath' => '/skins',
552552 'wgSitename' => 'MediaWiki',
553 - 'wgServerName' => 'Britney-Spears',
554553 'wgLanguageCode' => $lang,
555554 'wgDBprefix' => $wgDBtype != 'oracle' ? 'parsertest_' : 'pt_',
556555 'wgRawHtml' => isset( $opts['rawhtml'] ),
Index: trunk/phase3/maintenance/tests/parser/parserTests.txt
——@@ -2090,7 +2090,7 @@
20912091 !! input
20922092 {{SERVER}}
20932093 !! result
2094 -<p><a href="http://localhost" class="external free" rel="nofollow">http://localhost</a>
 2094+<p><a href="http://Britney-Spears" class="external free" rel="nofollow">http://Britney-Spears</a>
20952095 </p>
20962096 !! end
20972097
Index: trunk/phase3/docs/distributors.txt
——@@ -96,9 +96,10 @@
9797 intelligently:
9898
9999 * $wgEmergencyContact: An e-mail address that can be used to contact the wiki
100 - administrator. By default, "wikiadmin@$wgServerName".
 100+ administrator. By default, "wikiadmin@ServerName".
101101 * $wgPasswordSender: The e-mail address to use when sending password e-mails.
102 - By default, "MediaWiki Mail <apache@$wgServerName>".
 102+ By default, "MediaWiki Mail <apache@ServerName>".
 103+ (with ServerName guessed from the http request)
103104 * $wgSMTP: Can be configured to use SMTP for mail sending instead of PHP
104105 mail().
105106
Index: trunk/phase3/includes/parser/Parser.php
——@@ -2495,7 +2495,7 @@
24962496 * @private
24972497 */
24982498 function getVariableValue( $index, $frame=false ) {
2499 - global $wgContLang, $wgSitename, $wgServer, $wgServerName;
 2499+ global $wgContLang, $wgSitename, $wgServer;
25002500 global $wgArticlePath, $wgScriptPath, $wgStylePath;
25012501
25022502 /**
——@@ -2777,7 +2777,10 @@
27782778 case 'server':
27792779 return $wgServer;
27802780 case 'servername':
2781 - return $wgServerName;
 2781+ wfSuppressWarnings(); # May give an E_WARNING in PHP < 5.3.3
 2782+ $serverName = parse_url( $wgServer, PHP_URL_HOST );
 2783+ wfRestoreWarnings();
 2784+ return $serverName ? $serverName : $wgServer;
27822785 case 'scriptpath':
27832786 return $wgScriptPath;
27842787 case 'stylepath':
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -56,23 +56,23 @@
5757
5858 /** @cond file_level_code */
5959 if( isset( $_SERVER['SERVER_NAME'] ) ) {
60 - $wgServerName = $_SERVER['SERVER_NAME'];
 60+ $serverName = $_SERVER['SERVER_NAME'];
6161 } elseif( isset( $_SERVER['HOSTNAME'] ) ) {
62 - $wgServerName = $_SERVER['HOSTNAME'];
 62+ $serverName = $_SERVER['HOSTNAME'];
6363 } elseif( isset( $_SERVER['HTTP_HOST'] ) ) {
64 - $wgServerName = $_SERVER['HTTP_HOST'];
 64+ $serverName = $_SERVER['HTTP_HOST'];
6565 } elseif( isset( $_SERVER['SERVER_ADDR'] ) ) {
66 - $wgServerName = $_SERVER['SERVER_ADDR'];
 66+ $serverName = $_SERVER['SERVER_ADDR'];
6767 } else {
68 - $wgServerName = 'localhost';
 68+ $serverName = 'localhost';
6969 }
7070
7171 $wgProto = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') ? 'https' : 'http';
7272
73 -$wgServer = $wgProto.'://' . $wgServerName;
 73+$wgServer = $wgProto.'://' . $serverName;
7474 # If the port is a non-standard one, add it to the URL
7575 if( isset( $_SERVER['SERVER_PORT'] )
76 - && !strpos( $wgServerName, ':' )
 76+ && !strpos( $serverName, ':' )
7777 && ( ( $wgProto == 'http' && $_SERVER['SERVER_PORT'] != 80 )
7878 || ( $wgProto == 'https' && $_SERVER['SERVER_PORT'] != 443 ) ) ) {
7979
——@@ -968,15 +968,17 @@
969969 /**
970970 * Site admin email address.
971971 */
972 -$wgEmergencyContact = 'wikiadmin@' . $wgServerName;
 972+$wgEmergencyContact = 'wikiadmin@' . $serverName;
973973
974974 /**
975975 * Password reminder email address.
976976 *
977977 * The address we should use as sender when a user is requesting his password.
978978 */
979 -$wgPasswordSender = 'MediaWiki Mail <apache@' . $wgServerName . '>';
 979+$wgPasswordSender = 'MediaWiki Mail <apache@' . $serverName . '>';
980980
 981+unset($serverName); # Don't leak local variables to global scope
 982+
981983 /**
982984 * Dummy address which should be accepted during mail send action.
983985 * It might be necessary to adapt the address or to set it equal

Follow-up revisions

RevisionCommit summaryAuthorDate
r73957Remove $wgServerName in extensions, too (follow r73950)platonides17:05, 29 September 2010
r74165Release notes for r73950platonides22:17, 2 October 2010

Comments

#Comment by MaxSem (talk | contribs)   16:03, 29 September 2010

This variable is used in several extensions, including OAIRepo used on Wikimedia.

#Comment by Platonides (talk | contribs)   17:05, 29 September 2010

Extensions done in r73957.

#Comment by Raymond (talk | contribs)   07:26, 30 September 2010

This breaks every wiki with the extension ContactPage (used on nlwiki at least): 

Notice: Undefined variable: wgServerName in ...\LocalSettings.php on line ...

See installation hints on http://www.mediawiki.org/wiki/Extension:ContactPage too.


#Comment by Platonides (talk | contribs)   22:19, 2 October 2010

Documentation fixed.

I don't see it being used in InitialiseSettings.php or CommonSettings.php WMF usage doesn't seem affected.

Usage of $wgServerName for the domain wasn't a good idea, since you probably wanted user@example.org not user@www.example.org

#Comment by MZMcBride (talk | contribs)   20:32, 4 December 2010

This broke my test wiki (same ContactPage issue as above). I don't really see any virtue in removing this variable and it actively harms current installations of certain extensions.

Status & tagging log