(bug 38406) Properly quote table names in DatabaseBase::tableName()

Currently the logic is the following:
$database = ( $format == 'quoted' || $this->isQuotedIdentifier( $database ) ? $database : $this->addIdentifierQuotes( $database ) );
This means that $database will *not* be quoted if $format is set to 'quoted',
which is the opposite of what we want here. Instead I just copied the code
used to quote $table just below and replace the variable.

This causes SQL syntax errors when the shared table name contain hyphens
since it is not quoted.

No RELEASE-NOTES entry since this should be backported to 1.19 once merged.

Patchset #2: added some tests
Patchset #3: fix tests for SQLite

Change-Id: Idda844cb470454a9a22001a31b6692de49892ca2

diff --git a/includes/db/Database.php b/includes/db/Database.php
index cc4ffaf..18c39bc 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -2029,7 +2029,9 @@ abstract class DatabaseBase implements DatabaseType {
 
                # Quote the $database and $table and apply the prefix if not quoted.
                if ( isset( $database ) ) {
-                       $database = ( $format == 'quoted' || $this->isQuotedIdentifier( $database ) ? $database : $this->addIdentifierQuotes( $database ) );
+                       if ( $format == 'quoted' && !$this->isQuotedIdentifier( $database ) ) {
+                               $database = $this->addIdentifierQuotes( $database );
+                       }
                }
 
                $table = "{$prefix}{$table}";

diff --git a/tests/phpunit/includes/db/DatabaseTest.php b/tests/phpunit/includes/db/DatabaseTest.php
index 672e664..379ffb1 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseTest.php
+++ b/tests/phpunit/includes/db/DatabaseTest.php
@@ -57,6 +57,98 @@ class DatabaseTest extends MediaWikiTestCase {
                        $this->db->addQuotes( "string's cause trouble" ) );
        }
 
+       private function getSharedTableName( $table, $database, $prefix, $format = 'quoted' ) {
+               global $wgSharedDB, $wgSharedTables, $wgSharedPrefix;
+
+               $oldName = $wgSharedDB;
+               $oldTables = $wgSharedTables;
+               $oldPrefix = $wgSharedPrefix;
+
+               $wgSharedDB = $database;
+               $wgSharedTables = array( $table );
+               $wgSharedPrefix = $prefix;
+
+               $ret = $this->db->tableName( $table, $format );
+
+               $wgSharedDB = $oldName;
+               $wgSharedTables = $oldTables;
+               $wgSharedPrefix = $oldPrefix;
+
+               return $ret;
+       }
+
+       private function prefixAndQuote( $table, $database = null, $prefix = null, $format = 'quoted' ) {
+               if ( $this->db->getType() === 'sqlite' || $format !== 'quoted' ) {
+                       $quote = '';
+               } elseif ( $this->db->getType() === 'mysql' ) {
+                       $quote = '`';
+               } else {
+                       $quote = '"';
+               }
+
+               if ( $database !== null ) {
+                       $database = $quote . $database . $quote . '.';
+               }
+
+               if ( $prefix === null ) {
+                       $prefix = $this->dbPrefix();
+               }
+
+               return $database . $quote . $prefix . $table . $quote;
+       }
+
+       function testTableNameLocal() {
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename' ),
+                       $this->db->tableName( 'tablename' )
+               );
+       }
+
+       function testTableNameRawLocal() {
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', null, null, 'raw' ),
+                       $this->db->tableName( 'tablename', 'raw' )
+               );
+       }
+
+       function testTableNameShared() {
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', 'sharedatabase', 'sh_' ),
+                       $this->getSharedTableName( 'tablename', 'sharedatabase', 'sh_' )
+               );
+
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', 'sharedatabase', null ),
+                       $this->getSharedTableName( 'tablename', 'sharedatabase', null )
+               );
+       }
+
+       function testTableNameRawShared() {
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', 'sharedatabase', 'sh_', 'raw' ),
+                       $this->getSharedTableName( 'tablename', 'sharedatabase', 'sh_', 'raw' )
+               );
+
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', 'sharedatabase', null, 'raw' ),
+                       $this->getSharedTableName( 'tablename', 'sharedatabase', null, 'raw' )
+               );
+       }
+
+       function testTableNameForeign() {
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', 'databasename', '' ),
+                       $this->db->tableName( 'databasename.tablename' )
+               );
+       }
+
+       function testTableNameRawForeign() {
+               $this->assertEquals(
+                       $this->prefixAndQuote( 'tablename', 'databasename', '', 'raw' ),
+                       $this->db->tableName( 'databasename.tablename', 'raw' )
+               );
+       }
+
        function testFillPreparedEmpty() {
                $sql = $this->db->fillPrepared(
                        'SELECT * FROM interwiki', array() );