[BUGFIX] mssql: Identifier quoting and return types 18/53118/2
authorChristian Kuhn <lolli@schwarzbu.ch>
Sun, 4 Jun 2017 12:46:49 +0000 (14:46 +0200)
committerWouter Wolters <typo3@wouterwolters.nl>
Mon, 5 Jun 2017 09:47:42 +0000 (11:47 +0200)
Microsoft sql server field & columns quotes quotes identifiers as
[anIdentifier] in comparison to mysql and postgres which quote
with a character that is identical left and right.
The patch adapts some quoting methods to cope with that and
adapts a return type hint where the mssql doctrine driver returns
more precise value types than other platform drivers.

Change-Id: I8db6109d5a92ff43f3503f245c5d131b96201096
Resolves: #79297
Releases: master, 8.7
Reviewed-on: https://review.typo3.org/53118
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
typo3/sysext/core/Classes/Database/Query/Expression/ExpressionBuilder.php
typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php
typo3/sysext/core/Tests/Unit/Database/Query/QueryBuilderTest.php

index 692dd25..ba83359 100644 (file)
@@ -495,9 +495,9 @@ class ExpressionBuilder
      * @param mixed $input The parameter to be quoted.
      * @param string|null $type The type of the parameter.
      *
-     * @return string
+     * @return mixed Often string, but also int or float or similar depending on $input and platform
      */
-    public function literal($input, string $type = null): string
+    public function literal($input, string $type = null)
     {
         return $this->connection->quote($input, $type);
     }
index c820d03..8b2b553 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Core\Database\Query;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Query\Expression\CompositeExpression;
 use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
@@ -917,9 +918,9 @@ class QueryBuilder
      * @param mixed $input The parameter to be quoted.
      * @param string|null $type The type of the parameter.
      *
-     * @return string The quoted parameter.
+     * @return mixed Often string, but also int or float or similar depending on $input and platform
      */
-    public function quote($input, string $type = null): string
+    public function quote($input, string $type = null)
     {
         return $this->getConnection()->quote($input, $type);
     }
@@ -1018,13 +1019,18 @@ class QueryBuilder
      */
     protected function unquoteSingleIdentifier(string $identifier): string
     {
-        $quoteChar = $this->getConnection()
-            ->getDatabasePlatform()
-            ->getIdentifierQuoteCharacter();
-
-        $unquotedIdentifier = trim($identifier, $quoteChar);
-
-        return str_replace($quoteChar . $quoteChar, $quoteChar, $unquotedIdentifier);
+        $identifier = trim($identifier);
+        $platform = $this->getConnection()->getDatabasePlatform();
+        if ($platform instanceof SQLServerPlatform) {
+            // mssql quotes identifiers with [ and ], not a single character
+            $identifier = ltrim($identifier, '[');
+            $identifier = rtrim($identifier, ']');
+        } else {
+            $quoteChar = $platform->getIdentifierQuoteCharacter();
+            $identifier = trim($identifier, $quoteChar);
+            $identifier = str_replace($quoteChar . $quoteChar, $quoteChar, $identifier);
+        }
+        return $identifier;
     }
 
     /**
index 7ffc742..4190aba 100644 (file)
@@ -199,10 +199,13 @@ class ConnectionMigrator
             if ($createOnly) {
                 // Ignore new indexes that work on columns that need changes
                 foreach ($changedTable->addedIndexes as $indexName => $addedIndex) {
-                    // Strip MySQL prefix length information to get real column names
                     $indexColumns = array_map(
                         function ($columnName) {
-                            return preg_replace('/\(\d+\)$/', '', $columnName);
+                            // Strip MySQL prefix length information to get real column names
+                            $columnName = preg_replace('/\(\d+\)$/', '', $columnName);
+                            // Strip mssql '[' and ']' from column names
+                            $columnName = ltrim($columnName, '[');
+                            return rtrim($columnName, ']');
                         },
                         $addedIndex->getColumns()
                     );
index 4e0ed0f..df677e6 100644 (file)
@@ -15,6 +15,10 @@ namespace TYPO3\CMS\Core\Tests\Unit\Database\Query;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\MySqlPlatform;
+use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
+use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Prophecy\Argument;
 use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
@@ -22,8 +26,9 @@ use TYPO3\CMS\Core\Database\Query\QueryBuilder;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
 use TYPO3\CMS\Core\Tests\Unit\Database\Mocks\MockPlatform;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
-class QueryBuilderTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
+class QueryBuilderTest extends UnitTestCase
 {
     /**
      * @var Connection|\Prophecy\Prophecy\ObjectProphecy
@@ -31,7 +36,7 @@ class QueryBuilderTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     protected $connection;
 
     /**
-     * @var \Doctrine\DBAL\Platforms\AbstractPlatform
+     * @var AbstractPlatform
      */
     protected $platform;
 
@@ -1109,4 +1114,52 @@ class QueryBuilderTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
         ];
         $this->assertEquals($expected, $result);
     }
+
+    /**
+     * @return array
+     */
+    public function unquoteSingleIdentifierUnquotesCorrectlyOnDifferentPlatformsDataProvider()
+    {
+        return [
+            'mysql' => [
+                'platform' => MySqlPlatform::class,
+                'quoteChar' => '`',
+                'input' => '`anIdentifier`',
+                'expected' => 'anIdentifier',
+            ],
+            'mysql with spaces' => [
+                'platform' => MySqlPlatform::class,
+                'quoteChar' => '`',
+                'input' => ' `anIdentifier` ',
+                'expected' => 'anIdentifier',
+            ],
+            'postgres' => [
+                'platform' => PostgreSqlPlatform::class,
+                'quoteChar' => '"',
+                'input' => '"anIdentifier"',
+                'expected' => 'anIdentifier',
+            ],
+            'mssql' => [
+                'platform' => SQLServerPlatform::class,
+                'quoteChar' => '', // no single quote character, but [ and ]
+                'input' => '[anIdentifier]',
+                'expected' => 'anIdentifier',
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider unquoteSingleIdentifierUnquotesCorrectlyOnDifferentPlatformsDataProvider
+     */
+    public function unquoteSingleIdentifierUnquotesCorrectlyOnDifferentPlatforms($platform, $quoteChar, $input, $expected)
+    {
+        $connectionProphecy = $this->prophesize(Connection::class);
+        $databasePlatformProphecy = $this->prophesize($platform);
+        $databasePlatformProphecy->getIdentifierQuoteCharacter()->willReturn($quoteChar);
+        $connectionProphecy->getDatabasePlatform()->willReturn($databasePlatformProphecy);
+        $subject = GeneralUtility::makeInstance(QueryBuilder::class, $connectionProphecy->reveal());
+        $result = $this->callInaccessibleMethod($subject, 'unquoteSingleIdentifier', $input);
+        $this->assertEquals($expected, $result);
+    }
 }