From d12cb4ec1a014d1e2e22be18686af0ae23ffbb42 Mon Sep 17 00:00:00 2001 From: Morton Jonuschat <m.jonuschat@mojocode.de> Date: Mon, 27 Mar 2017 20:40:12 -0700 Subject: [PATCH] [TASK] Allow proper quoting of database identifiers in TypoScript 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> --- .../Classes/Database/Query/QueryHelper.php | 24 +++++++++ ...DbalCompatibleFieldQuotingInTypoScript.rst | 22 ++++++++ .../Fixtures/Frontend/JsonRenderer.ts | 18 ++----- .../Unit/Database/Query/QueryHelperTest.php | 52 +++++++++++++++++++ .../TypoScript/Helper/StylesContent.txt | 6 +-- .../Configuration/Setup/Index.rst | 2 +- .../Documentation/Installation/Index.rst | 2 +- .../InsertingContentPageTemplate/Index.rst | 4 +- .../Installation/Upgrading/Index.rst | 6 +-- .../ContentObject/ContentObjectRenderer.php | 5 +- .../Menu/AbstractMenuContentObject.php | 10 +++- .../Menu/AbstractMenuContentObjectTest.php | 48 +++++++++++++---- typo3/sysext/frontend/ext_localconf.php | 2 +- 13 files changed, 166 insertions(+), 35 deletions(-) create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Important-80506-DbalCompatibleFieldQuotingInTypoScript.rst diff --git a/typo3/sysext/core/Classes/Database/Query/QueryHelper.php b/typo3/sysext/core/Classes/Database/Query/QueryHelper.php index f50774e62175..bb18eb4a0b7c 100644 --- a/typo3/sysext/core/Classes/Database/Query/QueryHelper.php +++ b/typo3/sysext/core/Classes/Database/Query/QueryHelper.php @@ -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 index 000000000000..d96f4023e06f --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/master/Important-80506-DbalCompatibleFieldQuotingInTypoScript.rst @@ -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 diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts b/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts index 10d98527df86..38b1bb463f20 100644 --- a/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts +++ b/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts @@ -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] diff --git a/typo3/sysext/core/Tests/Unit/Database/Query/QueryHelperTest.php b/typo3/sysext/core/Tests/Unit/Database/Query/QueryHelperTest.php index b0ce7c69a430..c92f2960c3cc 100644 --- a/typo3/sysext/core/Tests/Unit/Database/Query/QueryHelperTest.php +++ b/typo3/sysext/core/Tests/Unit/Database/Query/QueryHelperTest.php @@ -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)); + } } diff --git a/typo3/sysext/css_styled_content/Configuration/TypoScript/Helper/StylesContent.txt b/typo3/sysext/css_styled_content/Configuration/TypoScript/Helper/StylesContent.txt index 5d79eab27dd0..1598fd559535 100644 --- a/typo3/sysext/css_styled_content/Configuration/TypoScript/Helper/StylesContent.txt +++ b/typo3/sysext/css_styled_content/Configuration/TypoScript/Helper/StylesContent.txt @@ -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 diff --git a/typo3/sysext/css_styled_content/Documentation/Configuration/Setup/Index.rst b/typo3/sysext/css_styled_content/Documentation/Configuration/Setup/Index.rst index e1e7a87fc8b4..a8f8600fcdeb 100644 --- a/typo3/sysext/css_styled_content/Documentation/Configuration/Setup/Index.rst +++ b/typo3/sysext/css_styled_content/Documentation/Configuration/Setup/Index.rst @@ -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 } diff --git a/typo3/sysext/css_styled_content/Documentation/Installation/Index.rst b/typo3/sysext/css_styled_content/Documentation/Installation/Index.rst index b19cf95680c4..4df0df8759d6 100644 --- a/typo3/sysext/css_styled_content/Documentation/Installation/Index.rst +++ b/typo3/sysext/css_styled_content/Documentation/Installation/Index.rst @@ -75,7 +75,7 @@ column into your template: table = tt_content select { orderBy = sorting - where = colPos=0 + where = {#colPos}=0 languageField = sys_language_uid } } diff --git a/typo3/sysext/fluid_styled_content/Documentation/Installation/InsertingContentPageTemplate/Index.rst b/typo3/sysext/fluid_styled_content/Documentation/Installation/InsertingContentPageTemplate/Index.rst index ccd55688c3df..5cfdcf4bc5fb 100644 --- a/typo3/sysext/fluid_styled_content/Documentation/Installation/InsertingContentPageTemplate/Index.rst +++ b/typo3/sysext/fluid_styled_content/Documentation/Installation/InsertingContentPageTemplate/Index.rst @@ -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 } } diff --git a/typo3/sysext/fluid_styled_content/Documentation/Installation/Upgrading/Index.rst b/typo3/sysext/fluid_styled_content/Documentation/Installation/Upgrading/Index.rst index 7528324e8480..a6f9af4c6c78 100644 --- a/typo3/sysext/fluid_styled_content/Documentation/Installation/Upgrading/Index.rst +++ b/typo3/sysext/fluid_styled_content/Documentation/Installation/Upgrading/Index.rst @@ -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 diff --git a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php index 39ee6b066091..7ec12d4cdab8 100644 --- a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php +++ b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php @@ -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(); diff --git a/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php b/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php index c516cde4344f..e395470bda5b 100644 --- a/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php +++ b/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php @@ -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 diff --git a/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php b/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php index 96ff6a21f781..9cf1321c4553 100644 --- a/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php +++ b/typo3/sysext/frontend/Tests/Unit/ContentObject/Menu/AbstractMenuContentObjectTest.php @@ -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; diff --git a/typo3/sysext/frontend/ext_localconf.php b/typo3/sysext/frontend/ext_localconf.php index 1a45b0a6643d..e9675d6d1acd 100644 --- a/typo3/sysext/frontend/ext_localconf.php +++ b/typo3/sysext/frontend/ext_localconf.php @@ -53,7 +53,7 @@ styles.content.get { table = tt_content select { orderBy = sorting - where = colPos=0 + where = {#colPos}=0 } } -- GitLab