Merge pull request #3317 from morozov/platforms-literal-escaping

Implemented proper escaping of string literals in platforms and schema managers
doctrine · Oct 7, 2018 · 94ba9d7 · 94ba9d7
2 parents 82fd5d6 + 423b03d
commit 94ba9d7
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 34 deletions.
diff --git a/lib/Doctrine/DBAL/Platforms/DrizzlePlatform.php b/lib/Doctrine/DBAL/Platforms/DrizzlePlatform.php
@@ -326,15 +326,15 @@ public function getListTablesSQL()
     public function getListTableColumnsSQL($table, $database = null)
     {
         if ($database) {
-            $database = "'" . $database . "'";
+            $databaseSQL = $this->quoteStringLiteral($database);
         } else {
-            $database = 'DATABASE()';
+            $databaseSQL = 'DATABASE()';
         }
 
         return 'SELECT COLUMN_NAME, DATA_TYPE, COLUMN_COMMENT, IS_NULLABLE, IS_AUTO_INCREMENT, CHARACTER_MAXIMUM_LENGTH, COLUMN_DEFAULT,' .
                ' NUMERIC_PRECISION, NUMERIC_SCALE, COLLATION_NAME' .
                ' FROM DATA_DICTIONARY.COLUMNS' .
-               ' WHERE TABLE_SCHEMA=' . $database . " AND TABLE_NAME = '" . $table . "'";
+               ' WHERE TABLE_SCHEMA=' . $databaseSQL . ' AND TABLE_NAME = ' . $this->quoteStringLiteral($table);
     }
 
     /**
@@ -343,14 +343,14 @@ public function getListTableColumnsSQL($table, $database = null)
     public function getListTableForeignKeysSQL($table, $database = null)
     {
         if ($database) {
-            $database = "'" . $database . "'";
+            $databaseSQL = $this->quoteStringLiteral($database);
         } else {
-            $database = 'DATABASE()';
+            $databaseSQL = 'DATABASE()';
         }
 
         return 'SELECT CONSTRAINT_NAME, CONSTRAINT_COLUMNS, REFERENCED_TABLE_NAME, REFERENCED_TABLE_COLUMNS, UPDATE_RULE, DELETE_RULE' .
                ' FROM DATA_DICTIONARY.FOREIGN_KEYS' .
-               ' WHERE CONSTRAINT_SCHEMA=' . $database . " AND CONSTRAINT_TABLE='" . $table . "'";
+               ' WHERE CONSTRAINT_SCHEMA=' . $databaseSQL . ' AND CONSTRAINT_TABLE=' . $this->quoteStringLiteral($table);
     }
 
     /**
@@ -359,14 +359,14 @@ public function getListTableForeignKeysSQL($table, $database = null)
     public function getListTableIndexesSQL($table, $database = null)
     {
         if ($database) {
-            $database = "'" . $database . "'";
+            $databaseSQL = $this->quoteStringLiteral($database);
         } else {
-            $database = 'DATABASE()';
+            $databaseSQL = 'DATABASE()';
         }
 
         return "SELECT INDEX_NAME AS 'key_name', COLUMN_NAME AS 'column_name', IS_USED_IN_PRIMARY AS 'primary', IS_UNIQUE=0 AS 'non_unique'" .
                ' FROM DATA_DICTIONARY.INDEX_PARTS' .
-               ' WHERE TABLE_SCHEMA=' . $database . " AND TABLE_NAME='" . $table . "'";
+               ' WHERE TABLE_SCHEMA=' . $databaseSQL . ' AND TABLE_NAME=' . $this->quoteStringLiteral($table);
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php b/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php
@@ -439,7 +439,7 @@ public function getCreateDatabaseSQL($name)
      */
     public function getDisallowDatabaseConnectionsSQL($database)
     {
-        return "UPDATE pg_database SET datallowconn = 'false' WHERE datname = '" . $database . "'";
+        return "UPDATE pg_database SET datallowconn = 'false' WHERE datname = " . $this->quoteStringLiteral($database);
     }
 
     /**

diff --git a/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php b/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php
@@ -27,7 +27,7 @@ class DB2SchemaManager extends AbstractSchemaManager
     public function listTableNames()
     {
         $sql  = $this->_platform->getListTablesSQL();
-        $sql .= " AND CREATOR = UPPER('" . $this->_conn->getUsername() . "')";
+        $sql .= ' AND CREATOR = UPPER(' . $this->_conn->quote($this->_conn->getUsername()) . ')';
 
         $tables = $this->_conn->fetchAll($sql);
 

diff --git a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php
@@ -117,15 +117,9 @@ public function listTableForeignKeys($table, $database = null)
         $tableForeignKeys = $this->_conn->fetchAll($sql);
 
         if (! empty($tableForeignKeys)) {
-            $createSql = $this->_conn->fetchAll(
-                sprintf(
-                    "SELECT sql FROM (SELECT * FROM sqlite_master UNION ALL SELECT * FROM sqlite_temp_master) WHERE type = 'table' AND name = '%s'",
-                    $table
-                )
-            );
-            $createSql = $createSql[0]['sql'] ?? '';
+            $createSql = $this->getCreateTableSQL($table);
 
-            if (preg_match_all(
+            if ($createSql !== null && preg_match_all(
                 '#
                     (?:CONSTRAINT\s+([^\s]+)\s+)?
                     (?:FOREIGN\s+KEY[^\)]+\)\s*)?
@@ -174,9 +168,10 @@ protected function _getPortableTableIndexesList($tableIndexes, $tableName = null
         $indexBuffer = [];
 
         // fetch primary
-        $stmt       = $this->_conn->executeQuery(
-            sprintf("PRAGMA TABLE_INFO ('%s')", $tableName)
-        );
+        $stmt       = $this->_conn->executeQuery(sprintf(
+            'PRAGMA TABLE_INFO (%s)',
+            $this->_conn->quote($tableName)
+        ));
         $indexArray = $stmt->fetchAll(FetchMode::ASSOCIATIVE);
 
         usort($indexArray, static function ($a, $b) {
@@ -212,10 +207,11 @@ protected function _getPortableTableIndexesList($tableIndexes, $tableName = null
             $idx['primary']    = false;
             $idx['non_unique'] = $tableIndex['unique']?false:true;
 
-            $stmt       = $this->_conn->executeQuery(
-                sprintf("PRAGMA INDEX_INFO ('%s')", $keyName)
-            );
-            $indexArray = $stmt->fetchAll(FetchMode::ASSOCIATIVE);
+                $stmt       = $this->_conn->executeQuery(sprintf(
+                    'PRAGMA INDEX_INFO (%s)',
+                    $this->_conn->quote($keyName)
+                ));
+                $indexArray = $stmt->fetchAll(FetchMode::ASSOCIATIVE);
 
             foreach ($indexArray as $indexColumnRow) {
                 $idx['column_name'] = $indexColumnRow['name'];
@@ -272,13 +268,7 @@ protected function _getPortableTableColumnList($table, $database, $tableColumns)
         }
 
         // inspect column collation and comments
-        $createSql = $this->_conn->fetchAll(
-            sprintf(
-                "SELECT sql FROM (SELECT * FROM sqlite_master UNION ALL SELECT * FROM sqlite_temp_master) WHERE type = 'table' AND name = '%s'",
-                $table
-            )
-        );
-        $createSql = $createSql[0]['sql'] ?? '';
+        $createSql = $this->getCreateTableSQL($table) ?? '';
 
         foreach ($list as $columnName => $column) {
             $type = $column->getType();
@@ -488,4 +478,24 @@ private function parseColumnCommentFromSQL(string $column, string $sql) : ?strin
 
         return $comment === '' ? null : $comment;
     }
+
+    private function getCreateTableSQL(string $table) : ?string
+    {
+        return $this->_conn->fetchColumn(
+            <<<'SQL'
+SELECT sql
+  FROM (
+      SELECT *
+        FROM sqlite_master
+   UNION ALL
+      SELECT *
+        FROM sqlite_temp_master
+  )
+WHERE type = 'table'
+AND name = ?
+SQL
+            ,
+            [$table]
+        ) ?: null;
+    }
 }
diff --git a/tests/Doctrine/Tests/DBAL/Schema/DB2SchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Schema/DB2SchemaManagerTest.php
@@ -30,7 +30,7 @@ protected function setUp()
         $platform      = $this->createMock(DB2Platform::class);
         $this->conn    = $this
             ->getMockBuilder(Connection::class)
-            ->setMethods(['fetchAll'])
+            ->setMethods(['fetchAll', 'quote'])
             ->setConstructorArgs([['platform' => $platform], $driverMock, new Configuration(), $eventManager])
             ->getMock();
         $this->manager = new DB2SchemaManager($this->conn);
@@ -102,6 +102,7 @@ public function testListTableNamesFiltersAssetNamesCorrectlyWithCallable()
         $this->conn->getConfiguration()->setSchemaAssetsFilter(static function ($assetName) use ($accepted) {
             return in_array($assetName, $accepted);
         });
+        $this->conn->expects($this->any())->method('quote');
         $this->conn->expects($this->once())->method('fetchAll')->will($this->returnValue([
             ['name' => 'FOO'],
             ['name' => 'T_FOO'],
@@ -129,6 +130,7 @@ public function testSettingNullExpressionWillResetCallable()
         $this->conn->getConfiguration()->setSchemaAssetsFilter(static function ($assetName) use ($accepted) {
             return in_array($assetName, $accepted);
         });
+        $this->conn->expects($this->any())->method('quote');
         $this->conn->expects($this->atLeastOnce())->method('fetchAll')->will($this->returnValue([
             ['name' => 'FOO'],
             ['name' => 'T_FOO'],
-Original file line number
+Diff line change
@@ Expand Up / @@ -439,7 +439,7 @@ public function getCreateDatabaseSQL($name) @@
          */
         public function getDisallowDatabaseConnectionsSQL($database)
         {
-            return "UPDATE pg_database SET datallowconn = 'false' WHERE datname = '" . $database . "'";
+            return "UPDATE pg_database SET datallowconn = 'false' WHERE datname = " . $this->quoteStringLiteral($database);
         }
         /**
@@ Expand Down @@