[TASK] Allow proper quoting of database identifiers in TypoScript 04/52204/5
authorMorton Jonuschat <m.jonuschat@mojocode.de>
Tue, 28 Mar 2017 03:40:12 +0000 (20:40 -0700)
committerChristian Kuhn <lolli@schwarzbu.ch>
Thu, 30 Mar 2017 19:56:30 +0000 (21:56 +0200)
Add markup to TypoScript CONTENT object options dealing with database
fields so that SQL fragments can be created in a DBMS agnostic way
using the proper quoting for the active database.

Parsing in `sortBy` and `groupBy` is disabled as these parameters
already follow a stricter syntax that allow automatic parsing and
quoting.

Usage Example: `select.where = {#colPos}=0`

Change-Id: I95592b82de08e6cb6f9e952e6c456417878c23a8
Resolves: #80506
Releases: master
Reviewed-on: https://review.typo3.org/52204
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
Reviewed-by: Daniel Goerz <ervaude@gmail.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
13 files changed:
typo3/sysext/core/Classes/Database/Query/QueryHelper.php
typo3/sysext/core/Documentation/Changelog/master/Important-80506-DbalCompatibleFieldQuotingInTypoScript.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts
typo3/sysext/core/Tests/Unit/Database/Query/QueryHelperTest.php
typo3/sysext/css_styled_content/Configuration/TypoScript/Helper/StylesContent.txt
typo3/sysext/css_styled_content/Documentation/Configuration/Setup/Index.rst
typo3/sysext/css_styled_content/Documentation/Installation/Index.rst
typo3/sysext/fluid_styled_content/Documentation/Installation/InsertingContentPageTemplate/Index.rst
typo3/sysext/fluid_styled_content/Documentation/Installation/Upgrading/Index.rst
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php
typo3/sysext/frontend/ext_localconf.php

index f50774e..bb18eb4 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Core\Database\Query;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
@@ -181,4 +182,27 @@ class QueryHelper
             ]
         ];
     }
+
+    /**
+     * Quote database table/column names indicated by {#identifier} markup in a SQL fragment string.
+     * This is an intermediate step to make SQL fragments in Typoscript and TCA database agnostic.
+     *
+     * @param \TYPO3\CMS\Core\Database\Connection $connection
+     * @param string $sql
+     * @return string
+     */
+    public static function quoteDatabaseIdentifiers(Connection $connection, string $sql): string
+    {
+        if (strpos($sql, '{#') !== false) {
+            $sql = preg_replace_callback(
+                '/{#(?P<identifier>[^}]+)}/',
+                function (array $matches) use ($connection) {
+                    return $connection->quoteIdentifier($matches['identifier']);
+                },
+                $sql
+            );
+        }
+
+        return $sql;
+    }
 }
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Important-80506-DbalCompatibleFieldQuotingInTypoScript.rst b/typo3/sysext/core/Documentation/Changelog/master/Important-80506-DbalCompatibleFieldQuotingInTypoScript.rst
new file mode 100644 (file)
index 0000000..d96f402
--- /dev/null
@@ -0,0 +1,22 @@
+.. include:: ../../Includes.txt
+
+===============================================================
+Important: #80506 - Dbal compatible field quoting in TypoScript
+===============================================================
+
+See :issue:`80506`
+
+Description
+===========
+
+Properties in :ts:`TypoScript` dealing with SQL fragments need proper quoting of field names to be compatible with different database drivers. The database framework of the core now applies proper quoting to field names if they are wrapped as :ts:`{#fieldName}`
+
+It is advised to adapt extensions accordingly to run successfully on databases like postgreSQL.
+
+Example for a select.where TypoScript snippet:
+
+.. code-block:: typoscript
+
+    select.where = {#colPos}=0
+
+.. index:: Database, Frontend, TypoScript
index 10d9852..38b1bb4 100644 (file)
@@ -75,7 +75,7 @@ page {
                 orderBy = sorting
                 where.field = uid
                 where.intval = 1
-                where.wrap = parenttable="pages" AND parentid=|
+                where.wrap = parenttable='pages' AND parentid=|
             }
             renderObj < lib.watcherDataObject
             renderObj.1.watcher.dataWrap = {register:watcher}|.tx_irretutorial_hotels/tx_irretutorial_1nff_hotel:{field:uid}
@@ -85,7 +85,7 @@ page {
                        table = tt_content
                        select {
                                orderBy = sorting
-                               where = colPos=0
+                               where = {#colPos}=0
                        }
                        renderObj < lib.watcherDataObject
                        renderObj.1.watcher.dataWrap = {register:watcher}|.__contents/tt_content:{field:uid}
@@ -115,7 +115,7 @@ page {
                                                orderBy = sorting
                                                where.field = uid
                                                where.intval = 1
-                                               where.wrap = parenttable="tt_content" AND parentid=|
+                                               where.wrap = parenttable='tt_content' AND parentid=|
                                        }
                                        renderObj < lib.watcherDataObject
                                        renderObj.1.watcher.dataWrap = {register:watcher}|.tx_irretutorial_1nff_hotels/tx_irretutorial_1nff_hotel:{field:uid}
@@ -128,7 +128,7 @@ page {
                                                                orderBy = sorting
                                                                where.field = uid
                                                                where.intval = 1
-                                                               where.wrap = parenttable="tx_irretutorial_1nff_hotel" AND parentid=|
+                                                               where.wrap = parenttable='tx_irretutorial_1nff_hotel' AND parentid=|
                                                        }
                                                        renderObj < lib.watcherDataObject
                                                        renderObj.1.watcher.dataWrap = {register:watcher}|.offers/tx_irretutorial_1nff_offer:{field:uid}
@@ -141,7 +141,7 @@ page {
                                                                                orderBy = sorting
                                                                                where.field = uid
                                                                                where.intval = 1
-                                                                               where.wrap = parenttable="tx_irretutorial_1nff_offer" AND parentid=|
+                                                                               where.wrap = parenttable='tx_irretutorial_1nff_offer' AND parentid=|
                                                                        }
                                                                        renderObj < lib.watcherDataObject
                                                                        renderObj.1.watcher.dataWrap = {register:watcher}|.prices/tx_irretutorial_1nff_price:{field:uid}
@@ -238,14 +238,6 @@ page {
        stdWrap.postUserFunc = TYPO3\TestingFramework\Core\Functional\Framework\Frontend\Renderer->renderSections
 }
 
-[globalVar = LIT:postgresql = {$databasePlatform}]
-page.10.15.select.where.wrap = "parenttable" = 'pages' AND parentid=|
-page.10.20.select.where = "colPos" = 0
-page.10.20.renderObj.20.select.where.wrap = "parenttable" = 'tt_content' AND "parentid" = |
-page.10.20.renderObj.20.renderObj.10.select.where.wrap = "parenttable" = 'tx_irretutorial_1nff_hotel' AND "parentid" = |
-page.10.20.renderObj.20.renderObj.10.renderObj.10.select.where.wrap = "parenttable" = 'tx_irretutorial_1nff_offer' AND "parentid" = |
-[end]
-
 [globalVar = GP:L = 1]
 config.sys_language_uid = 1
 [end]
index b0ce7c6..c92f296 100644 (file)
@@ -15,6 +15,8 @@ namespace TYPO3\CMS\Core\Tests\Unit\Database\Query;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Prophecy\Argument;
+use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\Query\QueryHelper;
 
 /**
@@ -347,4 +349,54 @@ class QueryHelperTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     {
         $this->assertSame($expected, QueryHelper::parseJoin($input));
     }
+
+    /**
+     * Test cases for quoting column/table name identifiers in SQL fragments
+     *
+     * @return array
+     */
+    public function quoteDatabaseIdentifierDataProvider(): array
+    {
+        return [
+            'no marked identifiers' => [
+                'colPos=0',
+                'colPos=0',
+            ],
+            'single fieldname' => [
+                '{#colPos}=0',
+                '"colPos"=0',
+            ],
+            'tablename and fieldname' => [
+                '{#tt_content.colPos}=0',
+                '"tt_content"."colPos"=0',
+            ],
+            'multiple fieldnames' => [
+                '{#colPos}={#aField}',
+                '"colPos"="aField"',
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider quoteDatabaseIdentifierDataProvider
+     * @param string $input
+     * @param string $expected
+     */
+    public function quoteDatabaseIdentifiers(string $input, string $expected)
+    {
+        $connectionProphet = $this->prophesize(Connection::class);
+        $connectionProphet->quoteIdentifier(Argument::cetera())->will(function ($args) {
+            $parts = array_map(
+                function ($identifier) {
+                    return '"' . $identifier . '"';
+                },
+                explode('.', $args[0])
+            );
+
+            return implode('.', $parts);
+        });
+
+        $this->assertSame($expected, QueryHelper::quoteDatabaseIdentifiers($connectionProphet->reveal(), $input));
+    }
 }
index 5d79eab..1598fd5 100644 (file)
@@ -1,14 +1,14 @@
 # get content, left
 styles.content.getLeft < styles.content.get
-styles.content.getLeft.select.where = colPos=1
+styles.content.getLeft.select.where = {#colPos}=1
 
 # get content, right
 styles.content.getRight < styles.content.get
-styles.content.getRight.select.where = colPos=2
+styles.content.getRight.select.where = {#colPos}=2
 
 # get content, margin
 styles.content.getBorder < styles.content.get
-styles.content.getBorder.select.where = colPos=3
+styles.content.getBorder.select.where = {#colPos}=3
 
 # get news
 styles.content.getNews < styles.content.get
index e1e7a87..a8f8600 100644 (file)
@@ -164,7 +164,7 @@ Here is some example setup code for :code:`styles.content`. Note that all proper
    styles.content.get {
            table = tt_content
            select.orderBy = sorting
-           select.where = colPos=0
+           select.where = {#colPos}=0
            select.languageField = sys_language_uid
    }
 
index b19cf95..4df0df8 100644 (file)
@@ -75,7 +75,7 @@ column into your template:
                            table = tt_content
                            select {
                                    orderBy = sorting
-                                   where = colPos=0
+                                   where = {#colPos}=0
                                    languageField = sys_language_uid
                            }
                    }
index ccd5568..5cfdcf4 100644 (file)
@@ -29,7 +29,7 @@ Based on the TEMPLATE content object (cObj)
                table = tt_content
                select {
                   orderBy = sorting
-                  where = colPos=0
+                  where = {#colPos}=0
                   languageField = sys_language_uid
                }
             }
@@ -56,7 +56,7 @@ Based on the FLUIDTEMPLATE content object (cObj)
                table = tt_content
                select {
                   orderBy = sorting
-                  where = colPos=0
+                  where = {#colPos}=0
                   languageField = sys_language_uid
                }
             }
index 7528324..a6f9af4 100644 (file)
@@ -47,15 +47,15 @@ The upgrade wizards can be found in the Install tool.
 
          # get content, left
          getLeft < styles.content.get
-         getLeft.select.where = colPos=1
+         getLeft.select.where = {#colPos}=1
 
          # get content, right
          getRight < styles.content.get
-         getRight.select.where = colPos=2
+         getRight.select.where = {#colPos}=2
 
          # get content, border
          getBorder < styles.content.get
-         getBorder.select.where = colPos=3
+         getBorder.select.where = {#colPos}=3
 
          # get news
          getNews < styles.content.get
index 39ee6b0..7ec12d4 100644 (file)
@@ -7398,6 +7398,7 @@ class ContentObjectRenderer
     public function getQuery($table, $conf, $returnQueryArray = false)
     {
         // Resolve stdWrap in these properties first
+        $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
         $properties = [
             'pidInList',
             'uidInList',
@@ -7420,6 +7421,8 @@ class ContentObjectRenderer
             );
             if ($conf[$property] === '') {
                 unset($conf[$property]);
+            } elseif (in_array($property, ['languageField', 'selectFields', 'join', 'leftJoin', 'rightJoin', 'where'], true)) {
+                $conf[$property] = QueryHelper::quoteDatabaseIdentifiers($connection, $conf[$property]);
             }
             if (isset($conf[$property . '.'])) {
                 // stdWrapping already done, so remove the sub-array
@@ -7481,7 +7484,7 @@ class ContentObjectRenderer
 
         $queryParts = $this->getQueryConstraints($table, $conf);
 
-        $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
+        $queryBuilder = $connection->createQueryBuilder();
         // @todo Check against getQueryConstraints, can probably use FrontendRestrictions
         // @todo here and remove enableFields there.
         $queryBuilder->getRestrictions()->removeAll();
index c516cde..e395470 100644 (file)
@@ -2190,8 +2190,16 @@ abstract class AbstractMenuContentObject
             'pidInList' => $pid,
             'orderBy' => $altSortField,
             'languageField' => 'sys_language_uid',
-            'where' => $useColPos >= 0 ? 'colPos=' . $useColPos : ''
+            'where' => ''
         ];
+
+        if ($useColPos >= 0) {
+            $expressionBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
+                ->getConnectionForTable('tt_content')
+                ->getExpressionBuilder();
+            $selectSetup['where'] = $expressionBuilder->eq('colPos', $useColPos);
+        }
+
         if ($basePageRow['content_from_pid']) {
             // If the page is configured to show content from a referenced page the sectionIndex contains only contents of
             // the referenced page
index 96ff6a2..9cf1321 100644 (file)
@@ -14,7 +14,13 @@ namespace TYPO3\CMS\Frontend\Tests\Unit\ContentObject\Menu;
  * The TYPO3 project - inspiring people to share!
  */
 use Doctrine\DBAL\Driver\Statement;
+use Prophecy\Argument;
 use TYPO3\CMS\Core\Cache\Frontend\VariableFrontend;
+use TYPO3\CMS\Core\Database\Connection;
+use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 use TYPO3\CMS\Frontend\ContentObject\Menu\AbstractMenuContentObject;
 
 /**
@@ -23,7 +29,12 @@ use TYPO3\CMS\Frontend\ContentObject\Menu\AbstractMenuContentObject;
 class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
 {
     /**
-     * @var \TYPO3\CMS\Frontend\ContentObject\Menu\AbstractMenuContentObject
+     * @var array
+     */
+    protected $singletonInstances = [];
+
+    /**
+     * @var AbstractMenuContentObject
      */
     protected $subject = null;
 
@@ -32,13 +43,24 @@ class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\Un
      */
     protected function setUp()
     {
-        $proxyClassName = $this->buildAccessibleProxy(\TYPO3\CMS\Frontend\ContentObject\Menu\AbstractMenuContentObject::class);
+        $proxyClassName = $this->buildAccessibleProxy(AbstractMenuContentObject::class);
         $this->subject = $this->getMockForAbstractClass($proxyClassName);
         $GLOBALS['TSFE'] = $this->getMockBuilder(\TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::class)
             ->setConstructorArgs([$GLOBALS['TYPO3_CONF_VARS'], 1, 1])
             ->getMock();
-        $GLOBALS['TSFE']->cObj = new \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer();
+        $GLOBALS['TSFE']->cObj = new ContentObjectRenderer();
         $GLOBALS['TSFE']->page = [];
+        $this->singletonInstances = GeneralUtility::getSingletonInstances();
+    }
+
+    /**
+     * Reset singleton instances
+     */
+    protected function tearDown()
+    {
+        GeneralUtility::purgeInstances();
+        GeneralUtility::resetSingletonInstances($this->singletonInstances);
+        parent::tearDown();
     }
 
     ////////////////////////////////
@@ -49,8 +71,16 @@ class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\Un
      */
     protected function prepareSectionIndexTest()
     {
+        $connectionProphet = $this->prophesize(Connection::class);
+        $connectionProphet->getExpressionBuilder()->willReturn(new ExpressionBuilder($connectionProphet->reveal()));
+        $connectionProphet->quoteIdentifier(Argument::cetera())->willReturnArgument(0);
+
+        $connectionPoolProphet = $this->prophesize(ConnectionPool::class);
+        $connectionPoolProphet->getConnectionForTable('tt_content')->willReturn($connectionProphet->reveal());
+        GeneralUtility::addInstance(ConnectionPool::class, $connectionPoolProphet->reveal());
+
         $this->subject->sys_page = $this->getMockBuilder(\TYPO3\CMS\Frontend\Page\PageRepository::class)->getMock();
-        $this->subject->parent_cObj = $this->getMockBuilder(\TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::class)->getMock();
+        $this->subject->parent_cObj = $this->getMockBuilder(ContentObjectRenderer::class)->getMock();
     }
 
     /**
@@ -176,11 +206,11 @@ class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\Un
         return [
             'no configuration' => [
                 [],
-                'colPos=0'
+                'colPos = 0'
             ],
             'with useColPos 2' => [
                 ['useColPos' => 2],
-                'colPos=2'
+                'colPos = 2'
             ],
             'with useColPos -1' => [
                 ['useColPos' => -1],
@@ -192,7 +222,7 @@ class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\Un
                         'wrap' => '2|'
                     ]
                 ],
-                'colPos=2'
+                'colPos = 2'
             ]
         ];
     }
@@ -279,7 +309,7 @@ class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\Un
         $this->subject = $this->getMockBuilder(AbstractMenuContentObject::class)->setMethods(['getRuntimeCache'])->getMockForAbstractClass();
         $this->subject->expects($this->once())->method('getRuntimeCache')->willReturn($runtimeCacheMock);
         $this->prepareSectionIndexTest();
-        $this->subject->parent_cObj = $this->getMockBuilder(\TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::class)->getMock();
+        $this->subject->parent_cObj = $this->getMockBuilder(ContentObjectRenderer::class)->getMock();
 
         $this->subject->sys_page->expects($this->once())->method('getMenu')->will($this->returnValue($menu));
         $this->subject->menuArr = [
@@ -530,7 +560,7 @@ class AbstractMenuContentObjectTest extends \TYPO3\TestingFramework\Core\Unit\Un
      */
     public function menuTypoLinkCreatesExpectedTypoLinkConfiguration(array $expected, array $mconf, $useCacheHash = true, array $page, $oTarget, $no_cache, $script, $overrideArray = '', $addParams = '', $typeOverride = '')
     {
-        $this->subject->parent_cObj = $this->getMockBuilder(\TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::class)
+        $this->subject->parent_cObj = $this->getMockBuilder(ContentObjectRenderer::class)
             ->setMethods(['typoLink'])
             ->getMock();
         $this->subject->mconf = $mconf;
index 1a45b0a..e9675d6 100644 (file)
@@ -53,7 +53,7 @@ styles.content.get {
     table = tt_content
     select {
         orderBy = sorting
-        where = colPos=0
+        where = {#colPos}=0
     }
 }