r41713 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r41712‎ | r41713 | r41714 >
Date:00:45, 6 October 2008
Author:tstarling
Status:old (Comments)
Tags:
Comment:
* Allow $wgDiff3=false
* Don't call quickUserCan('edit') unless section edit is enabled
* In DatabasePostgres and DatabaseSqlite: throw an exception on connection error
* In DatabasePostgres: don't send an invalid connection string whenever one of the fields is empty. Use quoting.
* In Database: make the captured PHP error prettier
* Display a descriptive error message when the user navigates to index.php with PHP 4, not a parse error. Check to see if the *.php5 extension works, using file_get_contents().
* The default port number for PostgreSQL is 5432, not blank.
* Better default for $wgDBname
Modified paths:
  • /trunk/phase3/extension-test.php5 (added) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/WebStart.php (modified) (history)
  • /trunk/phase3/includes/db/Database.php (modified) (history)
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/includes/templates/PHP4.php (added) (history)

Diff [purge]

Index: trunk/phase3/includes/GlobalFunctions.php
@@ -1270,7 +1270,7 @@
12711271
12721272 # This check may also protect against code injection in
12731273 # case of broken installations.
1274 - if(! file_exists( $wgDiff3 ) ){
 1274+ if( !$wgDiff3 || !file_exists( $wgDiff3 ) ) {
12751275 wfDebug( "diff3 not found\n" );
12761276 return false;
12771277 }
Index: trunk/phase3/includes/WebStart.php
@@ -74,6 +74,7 @@
7575 $IP = realpath( '.' );
7676 }
7777
 78+
7879 # Start profiler
7980 require_once( "$IP/StartProfiler.php" );
8081 wfProfileIn( 'WebStart.php-conf' );
@@ -81,20 +82,36 @@
8283 # Load up some global defines.
8384 require_once( "$IP/includes/Defines.php" );
8485
85 -# LocalSettings.php is the per site customization file. If it does not exit
86 -# the wiki installer need to be launched or the generated file moved from
87 -# ./config/ to ./
88 -if( !file_exists( "$IP/LocalSettings.php" ) ) {
89 - require_once( "$IP/includes/DefaultSettings.php" ); # used for printing the version
90 - require_once( "$IP/includes/templates/NoLocalSettings.php" );
91 - die();
 86+# Check for PHP 5
 87+if ( !function_exists( 'version_compare' )
 88+ || version_compare( phpversion(), '5.0.0' ) < 0
 89+) {
 90+ define( 'MW_PHP4', '1' );
 91+ require( "$IP/includes/DefaultSettings.php" );
 92+ require( "$IP/includes/templates/PHP4.php" );
 93+ exit;
9294 }
9395
9496 # Start the autoloader, so that extensions can derive classes from core files
9597 require_once( "$IP/includes/AutoLoader.php" );
9698
97 -# Include site settings. $IP may be changed (hopefully before the AutoLoader is invoked)
98 -require_once( "$IP/LocalSettings.php" );
 99+if ( defined( 'MW_CONFIG_CALLBACK' ) ) {
 100+ # Use a callback function to configure MediaWiki
 101+ require_once( "$IP/includes/DefaultSettings.php" );
 102+ call_user_func( MW_CONFIG_CALLBACK );
 103+} else {
 104+ # LocalSettings.php is the per site customization file. If it does not exit
 105+ # the wiki installer need to be launched or the generated file moved from
 106+ # ./config/ to ./
 107+ if( !file_exists( "$IP/LocalSettings.php" ) ) {
 108+ require_once( "$IP/includes/DefaultSettings.php" ); # used for printing the version
 109+ require_once( "$IP/includes/templates/NoLocalSettings.php" );
 110+ die();
 111+ }
 112+
 113+ # Include site settings. $IP may be changed (hopefully before the AutoLoader is invoked)
 114+ require_once( "$IP/LocalSettings.php" );
 115+}
99116 wfProfileOut( 'WebStart.php-conf' );
100117
101118 wfProfileIn( 'WebStart.php-ob_start' );
Index: trunk/phase3/includes/parser/Parser.php
@@ -3445,10 +3445,11 @@
34463446 global $wgMaxTocLevel, $wgContLang;
34473447
34483448 $doNumberHeadings = $this->mOptions->getNumberHeadings();
3449 - if( !$this->mTitle->quickUserCan( 'edit' ) ) {
 3449+ $showEditLink = $this->mOptions->getEditSection();
 3450+
 3451+ // Do not call quickUserCan unless necessary
 3452+ if( $showEditLink && !$this->mTitle->quickUserCan( 'edit' ) ) {
34503453 $showEditLink = 0;
3451 - } else {
3452 - $showEditLink = $this->mOptions->getEditSection();
34533454 }
34543455
34553456 # Inhibit editsection links if requested in the page
Index: trunk/phase3/includes/templates/PHP4.php
@@ -0,0 +1,87 @@
 2+<?php
 3+/**
 4+ * @file
 5+ * @ingroup Templates
 6+ */
 7+
 8+if( isset( $_SERVER['REQUEST_URI'] ) ) {
 9+ $scriptUrl = $_SERVER['REQUEST_URI'];
 10+} elseif( isset( $_SERVER['SCRIPT_NAME'] ) ) {
 11+ // Probably IIS; doesn't set REQUEST_URI
 12+ $scriptUrl = $_SERVER['SCRIPT_NAME'];
 13+} else {
 14+ $scriptUrl = '';
 15+}
 16+if ( preg_match( '!^(.*)/config/[^/]*.php$!', $scriptUrl, $m ) ) {
 17+ $baseUrl = $m[1];
 18+} elseif ( preg_match( '!^(.*)/[^/]*.php$!', $scriptUrl, $m ) ) {
 19+ $baseUrl = $m[1];
 20+} else {
 21+ $baseUrl = dirname( $baseUrl );
 22+}
 23+
 24+?>
 25+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
 26+<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>
 27+ <head>
 28+ <title>MediaWiki <?php echo htmlspecialchars( $wgVersion ); ?></title>
 29+ <meta http-equiv='Content-Type' content='text/html; charset=utf-8' />
 30+ <style type='text/css' media='screen, projection'>
 31+ html, body {
 32+ color: #000;
 33+ background-color: #fff;
 34+ font-family: sans-serif;
 35+ text-align: center;
 36+ }
 37+
 38+ p {
 39+ text-align: left;
 40+ margin-left: 2em;
 41+ margin-right: 2em;
 42+ }
 43+
 44+ h1 {
 45+ font-size: 150%;
 46+ }
 47+ </style>
 48+ </head>
 49+ <body>
 50+ <img src="<?php echo htmlspecialchars( $baseUrl ) ?>/skins/common/images/mediawiki.png" alt='The MediaWiki logo' />
 51+
 52+ <h1>MediaWiki <?php echo htmlspecialchars( $wgVersion ); ?></h1>
 53+ <div class='error'>
 54+<p>
 55+ MediaWiki requires PHP 5.0.0 or higher. You are running PHP
 56+ <?php echo htmlspecialchars( phpversion() ); ?>.
 57+</p>
 58+<?php
 59+flush();
 60+/**
 61+ * Test the *.php5 extension
 62+ */
 63+$downloadOther = true;
 64+if ( $baseUrl ) {
 65+ $testUrl = "$wgServer$baseUrl/extension-test.php5";
 66+ ini_set( 'allow_url_fopen', '1' );
 67+ $s = file_get_contents( $testUrl );
 68+
 69+ if ( strpos( $s, 'yes' ) !== false ) {
 70+ $encUrl = htmlspecialchars( str_replace( '.php', '.php5', $scriptUrl ) );
 71+ echo "<p>You may be able to use MediaWiki using a <a href=\"$encUrl\">.php5</a> file extension.</p>";
 72+ $downloadOther = false;
 73+ }
 74+}
 75+if ( $downloadOther ) {
 76+?>
 77+<p>Please consider upgrading your copy of PHP. PHP 4 is at the end of its
 78+lifecycle and will not receive further security updates.</p>
 79+<p>If for some reason you really really need to run MediaWiki on PHP 4, you will need to
 80+<a href="http://www.mediawiki.org/wiki/Download">download version 1.6.x</a>
 81+from our website. </p>
 82+<?php
 83+}
 84+?>
 85+
 86+ </div>
 87+ </body>
 88+</html>
Property changes on: trunk/phase3/includes/templates/PHP4.php
___________________________________________________________________
Added: svn:eol-style
189 + native
Index: trunk/phase3/includes/db/DatabasePostgres.php
@@ -78,12 +78,6 @@
7979 $failFunction = false, $flags = 0 )
8080 {
8181
82 - global $wgOut;
83 - # Can't get a reference if it hasn't been set yet
84 - if ( !isset( $wgOut ) ) {
85 - $wgOut = NULL;
86 - }
87 - $this->mOut =& $wgOut;
8882 $this->mFailFunction = $failFunction;
8983 $this->mFlags = $flags;
9084 $this->open( $server, $user, $password, $dbName);
@@ -138,10 +132,6 @@
139133
140134 global $wgDBport;
141135
142 - if (!strlen($user)) { ## e.g. the class is being loaded
143 - return;
144 - }
145 -
146136 $this->close();
147137 $this->mServer = $server;
148138 $this->mPort = $port = $wgDBport;
@@ -149,22 +139,31 @@
150140 $this->mPassword = $password;
151141 $this->mDBname = $dbName;
152142
153 - $hstring="";
 143+ $connectVars = array(
 144+ 'dbname' => $dbName,
 145+ 'user' => $user,
 146+ 'password' => $password );
154147 if ($server!=false && $server!="") {
155 - $hstring="host=$server ";
 148+ $connectVars['host'] = $server;
156149 }
157150 if ($port!=false && $port!="") {
158 - $hstring .= "port=$port ";
 151+ $connectVars['port'] = $port;
159152 }
 153+ $connectString = $this->makeConnectionString( $connectVars );
160154
161 - error_reporting( E_ALL );
162 - @$this->mConn = pg_connect("$hstring dbname=$dbName user=$user password=$password");
 155+ $this->installErrorHandler();
 156+ $this->mConn = pg_connect( $connectString );
 157+ $phpError = $this->restoreErrorHandler();
163158
164159 if ( $this->mConn == false ) {
165160 wfDebug( "DB connection error\n" );
166161 wfDebug( "Server: $server, Database: $dbName, User: $user, Password: " . substr( $password, 0, 3 ) . "...\n" );
167162 wfDebug( $this->lastError()."\n" );
168 - return false;
 163+ if ( !$this->mFailFunction ) {
 164+ throw new DBConnectionError( $this, $phpError );
 165+ } else {
 166+ return false;
 167+ }
169168 }
170169
171170 $this->mOpened = true;
@@ -189,7 +188,15 @@
190189 return $this->mConn;
191190 }
192191
 192+ function makeConnectionString( $vars ) {
 193+ $s = '';
 194+ foreach ( $vars as $name => $value ) {
 195+ $s .= "$name='" . str_replace( "'", "\\'", $value ) . "' ";
 196+ }
 197+ return $s;
 198+ }
193199
 200+
194201 function initial_setup($password, $dbName) {
195202 // If this is the initial connection, setup the schema stuff and possibly create the user
196203 global $wgDBname, $wgDBuser, $wgDBpassword, $wgDBsuperuser, $wgDBmwschema, $wgDBts2schema;
@@ -197,8 +204,8 @@
198205 print "<li>Checking the version of Postgres...";
199206 $version = $this->getServerVersion();
200207 $PGMINVER = '8.1';
201 - if ($this->numeric_version < $PGMINVER) {
202 - print "<b>FAILED</b>. Required version is $PGMINVER. You have $this->numeric_version ($version)</li>\n";
 208+ if ($version < $PGMINVER) {
 209+ print "<b>FAILED</b>. Required version is $PGMINVER. You have $version</li>\n";
203210 dieout("</ul>");
204211 }
205212 print "version $this->numeric_version is OK.</li>\n";
@@ -729,8 +736,7 @@
730737
731738 $table = $this->tableName( $table );
732739 if (! isset( $wgDBversion ) ) {
733 - $this->getServerVersion();
734 - $wgDBversion = $this->numeric_version;
 740+ $wgDBversion = $this->getServerVersion();
735741 }
736742
737743 if ( !is_array( $options ) )
@@ -1046,17 +1052,10 @@
10471053 * @return string Version information from the database
10481054 */
10491055 function getServerVersion() {
1050 - $version = pg_fetch_result($this->doQuery("SELECT version()"),0,0);
1051 - $thisver = array();
1052 - if (!preg_match('/PostgreSQL (\d+\.\d+)(\S+)/', $version, $thisver)) {
1053 - die("Could not determine the numeric version from $version!");
1054 - }
1055 - $version = $thisver[1].$thisver[2];
1056 - $this->numeric_version = $thisver[1];
1057 - return $version;
 1056+ $versionInfo = pg_version( $this->mConn );
 1057+ return $versionInfo['server'];
10581058 }
10591059
1060 -
10611060 /**
10621061 * Query whether a given relation exists (in the given schema, or the
10631062 * default mw one if not given)
Index: trunk/phase3/includes/db/Database.php
@@ -423,12 +423,22 @@
424424
425425 protected function installErrorHandler() {
426426 $this->mPHPError = false;
 427+ $this->htmlErrors = ini_set( 'html_errors', '0' );
427428 set_error_handler( array( $this, 'connectionErrorHandler' ) );
428429 }
429430
430431 protected function restoreErrorHandler() {
431432 restore_error_handler();
432 - return $this->mPHPError;
 433+ if ( $this->htmlErrors !== false ) {
 434+ ini_set( 'html_errors', $this->htmlErrors );
 435+ }
 436+ if ( $this->mPHPError ) {
 437+ $error = preg_replace( '!\[<a.*</a>\]!', '', $this->mPHPError );
 438+ $error = preg_replace( '!^.*?:(.*)$!', '$1', $error );
 439+ return $error;
 440+ } else {
 441+ return false;
 442+ }
433443 }
434444
435445 protected function connectionErrorHandler( $errno, $errstr ) {
Index: trunk/phase3/includes/db/DatabaseSqlite.php
@@ -23,8 +23,6 @@
2424 global $wgOut,$wgSQLiteDataDir, $wgSQLiteDataDirMode;
2525 if ("$wgSQLiteDataDir" == '') $wgSQLiteDataDir = dirname($_SERVER['DOCUMENT_ROOT']).'/data';
2626 if (!is_dir($wgSQLiteDataDir)) wfMkdirParents( $wgSQLiteDataDir, $wgSQLiteDataDirMode );
27 - if (!isset($wgOut)) $wgOut = NULL; # Can't get a reference if it hasn't been set yet
28 - $this->mOut =& $wgOut;
2927 $this->mFailFunction = $failFunction;
3028 $this->mFlags = $flags;
3129 $this->mDatabaseFile = "$wgSQLiteDataDir/$dbName.sqlite";
@@ -48,11 +46,28 @@
4947 $this->mConn = false;
5048 if ($dbName) {
5149 $file = $this->mDatabaseFile;
52 - if ($this->mFlags & DBO_PERSISTENT) $this->mConn = new PDO("sqlite:$file",$user,$pass,array(PDO::ATTR_PERSISTENT => true));
53 - else $this->mConn = new PDO("sqlite:$file",$user,$pass);
54 - if ($this->mConn === false) wfDebug("DB connection error: $err\n");;
 50+ try {
 51+ if ( $this->mFlags & DBO_PERSISTENT ) {
 52+ $this->mConn = new PDO( "sqlite:$file", $user, $pass,
 53+ array( PDO::ATTR_PERSISTENT => true ) );
 54+ } else {
 55+ $this->mConn = new PDO( "sqlite:$file", $user, $pass );
 56+ }
 57+ } catch ( PDOException $e ) {
 58+ $err = $e->getMessage();
 59+ }
 60+ if ( $this->mConn === false ) {
 61+ wfDebug( "DB connection error: $err\n" );
 62+ if ( !$this->mFailFunction ) {
 63+ throw new DBConnectionError( $this, $err );
 64+ } else {
 65+ return false;
 66+ }
 67+
 68+ }
5569 $this->mOpened = $this->mConn;
56 - $this->mConn->setAttribute(PDO::ATTR_ERRMODE,PDO::ERRMODE_SILENT); # set error codes only, dont raise exceptions
 70+ # set error codes only, don't raise exceptions
 71+ $this->mConn->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT );
5772 }
5873 return $this->mConn;
5974 }
Index: trunk/phase3/includes/DefaultSettings.php
@@ -27,8 +27,10 @@
2828 * Create a site configuration object
2929 * Not used for much in a default install
3030 */
31 -require_once( "$IP/includes/SiteConfiguration.php" );
32 -$wgConf = new SiteConfiguration;
 31+if ( !defined( 'MW_PHP4' ) ) {
 32+ require_once( "$IP/includes/SiteConfiguration.php" );
 33+ $wgConf = new SiteConfiguration;
 34+}
3335
3436 /** MediaWiki version number */
3537 $wgVersion = '1.14alpha';
@@ -539,10 +541,10 @@
540542 */
541543 /** database host name or ip address */
542544 $wgDBserver = 'localhost';
543 -/** database port number */
544 -$wgDBport = '';
 545+/** database port number (for PostgreSQL) */
 546+$wgDBport = 5432;
545547 /** name of the database */
546 -$wgDBname = 'wikidb';
 548+$wgDBname = 'my_wiki';
547549 /** */
548550 $wgDBconnection = '';
549551 /** Database username */
Index: trunk/phase3/extension-test.php5
@@ -0,0 +1,11 @@
 2+<?php
 3+
 4+/**
 5+ * Test for *.php5 capability in webserver
 6+ * Used by includes/templates/PHP4.php
 7+ */
 8+if ( version_compare( phpversion(), '5.0.0' ) >= 0 ) {
 9+ echo 'y'.'e'.'s';
 10+}
 11+
 12+?>

Follow-up revisions

RevisionCommit summaryAuthorDate
r41791Cleanup for r41713 PHP 4 warning page:...brion01:10, 7 October 2008

Comments

#Comment by Brion VIBBER (talk | contribs)   00:39, 7 October 2008

The PHP 4 template spews a bunch of undefined variable warnings; vulnerable to register_globals XSS at least.

#Comment by Brion VIBBER (talk | contribs)   01:10, 7 October 2008

Cleaned that up in r41791.

#Comment by Brion VIBBER (talk | contribs)   01:22, 7 October 2008

A further general issue with PHP 4... if index.php5 does work, it'd be nice to auto-forward the victim from index.php to index.php5. Just something to consider. :)

#Comment by Tim Starling (talk | contribs)   04:28, 10 October 2008

I don't see any XSS.

Status & tagging log