From 2297bf1776933a42c1c71944aa6b3f061206b098 Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Sun, 4 Jun 2017 14:46:49 +0200
Subject: [PATCH] [BUGFIX] mssql: Identifier quoting and return types

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/53115
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
---
 .../Query/Expression/ExpressionBuilder.php    |  4 +-
 .../Classes/Database/Query/QueryBuilder.php   | 24 +++++---
 .../Database/Schema/ConnectionMigrator.php    |  7 ++-
 .../Unit/Database/Query/QueryBuilderTest.php  | 57 ++++++++++++++++++-
 4 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/typo3/sysext/core/Classes/Database/Query/Expression/ExpressionBuilder.php b/typo3/sysext/core/Classes/Database/Query/Expression/ExpressionBuilder.php
index 692dd25b795a..ba8335925df1 100644
--- a/typo3/sysext/core/Classes/Database/Query/Expression/ExpressionBuilder.php
+++ b/typo3/sysext/core/Classes/Database/Query/Expression/ExpressionBuilder.php
@@ -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);
     }
diff --git a/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php b/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
index c820d033a0fd..8b2b5538fd85 100644
--- a/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
+++ b/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
@@ -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;
     }
 
     /**
diff --git a/typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php b/typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php
index 7ffc742172c7..4190abaca969 100644
--- a/typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php
+++ b/typo3/sysext/core/Classes/Database/Schema/ConnectionMigrator.php
@@ -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()
                     );
diff --git a/typo3/sysext/core/Tests/Unit/Database/Query/QueryBuilderTest.php b/typo3/sysext/core/Tests/Unit/Database/Query/QueryBuilderTest.php
index 4e0ed0f86fc4..df677e6f2606 100644
--- a/typo3/sysext/core/Tests/Unit/Database/Query/QueryBuilderTest.php
+++ b/typo3/sysext/core/Tests/Unit/Database/Query/QueryBuilderTest.php
@@ -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);
+    }
 }
-- 
GitLab