Fix a bug in the installer caused by r77487 creating installer sql statements like...
authorDaniel Friesen <dantman@users.mediawiki.org>
Sat, 4 Dec 2010 09:27:02 +0000 (09:27 +0000)
committerDaniel Friesen <dantman@users.mediawiki.org>
Sat, 4 Dec 2010 09:27:02 +0000 (09:27 +0000)
* Drop unused and likely broken /*$var*/` -> `$var syntax
* Replace {$var} with '{$var}' and `{$var}` handling that uses relevant database independent quoting ({$var} without surrouding quotes are never used)
* Give the generic/mysql class a proper quote_ident implementation
* Fix the unused Oracle and Sqlite quote_ident implementations which are potential sql injections if used
* Split common variable replacemnt code off to a replaceGlobalVars and make the generic and oracle code use it instead of duplicating the same code as each other

includes/db/Database.php
includes/db/DatabaseOracle.php
includes/db/DatabasePostgres.php
includes/db/DatabaseSqlite.php

index b20f461..e74d06e 100644 (file)
@@ -1694,6 +1694,15 @@ abstract class DatabaseBase implements DatabaseType {
                }
        }
 
+       /**
+        * Quotes a string using `backticks` for things like database, table, and field
+        * names, other databases which use something other than backticks can replace
+        * this with something else
+        */      
+       function quote_ident( $s ) {
+               return "`" . $this->strencode( $s ) . "`";
+       }
+
        /**
         * Escape string for safe LIKE usage.
         * WARNING: you should almost never use this function directly,
@@ -2500,6 +2509,32 @@ abstract class DatabaseBase implements DatabaseType {
                return true;
        }
 
+       /**
+        * Database independent variable replacement, replaces a set of named variables
+        * in a sql statement with the contents of their global variables.
+        * Supports '{$var}' `{$var}` and / *$var* / (without the spaces) style variables
+        * 
+        * '{$var}' should be used for text and is passed through the database's addQuotes method
+        * `{$var}` should be used for identifiers (eg: table and database names), it is passed through
+        *          the database's quote_ident method which can be overridden if the database
+        *          uses something other than backticks.
+        * / *$var* / is just encoded, besides traditional dbprefix and tableoptions it's use should be avoided
+        * 
+        * @param $ins String: SQL statement to replace variables in
+        * @param $varnames Array: Array of global variable names to replace
+        * @return String The new SQL statement with variables replaced
+        */
+       protected function replaceGlobalVars( $ins, $varnames ) {
+               foreach ( $varnames as $var ) {
+                       if ( isset( $GLOBALS[$var] ) ) {
+                               $ins = str_replace( '\'{$' . $var . '}\'', $this->addQuotes( $GLOBALS[$var] ), $ins ); // replace '{$var}'
+                               $ins = str_replace( '`{$' . $var . '}`', $this->quote_ident( $GLOBALS[$var] ), $ins ); // replace `{$var}`
+                               $ins = str_replace( '/*$' . $var . '*/', $this->strencode( $GLOBALS[$var] ) , $ins ); // replace /*$var*/
+                       }
+               }
+               return $ins;
+       }
+
        /**
         * Replace variables in sourced SQL
         */
@@ -2510,15 +2545,7 @@ abstract class DatabaseBase implements DatabaseType {
                        'wgDBadminuser', 'wgDBadminpassword', 'wgDBTableOptions',
                );
 
-               // Ordinary variables
-               foreach ( $varnames as $var ) {
-                       if ( isset( $GLOBALS[$var] ) ) {
-                               $val = $this->addQuotes( $GLOBALS[$var] ); // FIXME: safety check?
-                               $ins = str_replace( '{$' . $var . '}', $val, $ins );
-                               $ins = str_replace( '/*$' . $var . '*/`', '`' . $val, $ins );
-                               $ins = str_replace( '/*$' . $var . '*/', $val, $ins );
-                       }
-               }
+               $ins = $this->replaceGlobalVars( $ins, $varnames );
 
                // Table prefixes
                $ins = preg_replace_callback( '!/\*(?:\$wgDBprefix|_)\*/([a-zA-Z_0-9]*)!',
index 42b798e..876188d 100644 (file)
@@ -1126,7 +1126,7 @@ class DatabaseOracle extends DatabaseBase {
        }
 
        function quote_ident( $s ) {
-               return $s;
+               return '"' . str_replace( '"', '""', $s ) . '"';
        }
 
        function selectRow( $table, $vars, $conds, $fname = 'DatabaseOracle::selectRow', $options = array(), $join_conds = array() ) {
@@ -1345,15 +1345,7 @@ class DatabaseOracle extends DatabaseBase {
                        $varnames[] = '_OracleTempTS';
                }
 
-               // Ordinary variables
-               foreach ( $varnames as $var ) {
-                       if ( isset( $GLOBALS[$var] ) ) {
-                               $val = $this->addQuotes( $GLOBALS[$var] ); // FIXME: safety check?
-                               $ins = str_replace( '{$' . $var . '}', $val, $ins );
-                               $ins = str_replace( '/*$' . $var . '*/`', '`' . $val, $ins );
-                               $ins = str_replace( '/*$' . $var . '*/', $val, $ins );
-                       }
-               }
+               $ins = $this->replaceGlobalVars( $ins, $varnames );
 
                return parent::replaceVars( $ins );
        }
index e651e94..ea768e2 100644 (file)
@@ -973,7 +973,7 @@ class DatabasePostgres extends DatabaseBase {
         * Return the next in a sequence, save the value for retrieval via insertId()
         */
        function nextSequenceValue( $seqName ) {
-               $safeseq = preg_replace( "/'/", "''", $seqName );
+               $safeseq = str_replace( "'", "''", $seqName );
                $res = $this->query( "SELECT nextval('$safeseq')" );
                $row = $this->fetchRow( $res );
                $this->mInsertId = $row[0];
@@ -984,7 +984,7 @@ class DatabasePostgres extends DatabaseBase {
         * Return the current value of a sequence. Assumes it has been nextval'ed in this session.
         */
        function currentSequenceValue( $seqName ) {
-               $safeseq = preg_replace( "/'/", "''", $seqName );
+               $safeseq = str_replace( "'", "''", $seqName );
                $res = $this->query( "SELECT currval('$safeseq')" );
                $row = $this->fetchRow( $res );
                $currval = $row[0];
@@ -1242,7 +1242,7 @@ SQL;
         * Query whether a given schema exists. Returns the name of the owner
         */
        function schemaExists( $schema ) {
-               $eschema = preg_replace( "/'/", "''", $schema );
+               $eschema = str_replace( "'", "''", $schema );
                $SQL = "SELECT rolname FROM pg_catalog.pg_namespace n, pg_catalog.pg_roles r "
                                ."WHERE n.nspowner=r.oid AND n.nspname = '$eschema'";
                $res = $this->query( $SQL );
@@ -1301,7 +1301,7 @@ SQL;
        }
 
        function quote_ident( $s ) {
-               return '"' . preg_replace( '/"/', '""', $s ) . '"';
+               return '"' . str_replace( '"', '""', $s ) . '"';
        }
 
        /**
index e29e1a4..5bc36ce 100644 (file)
@@ -531,7 +531,7 @@ class DatabaseSqlite extends DatabaseBase {
        }
 
        function quote_ident( $s ) {
-               return $s;
+               return '"' . str_replace( '"', '""', $s ) . '"';
        }
 
        function buildLike() {