From 8bc6fc8b2219d96231d78d3f7dd1e0e8224a0f72 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Tue, 12 Mar 2024 00:08:58 +0100
Subject: [PATCH] [TASK] Remove leftover query parsing code in cObj
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Some code in ContentObjectRenderer is not needed
anymore as the query parts are not parsed anymore.
Resolves: #103371
Releases: main
Change-Id: Ie313c7006d2f103c48c9bac97abbd4343d277052
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83408
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Andreas Kienast <a.fernandez@scripting-base.de>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Andreas Kienast <a.fernandez@scripting-base.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Benni Mack <benni@typo3.org>
---
.../Classes/Database/Query/QueryBuilder.php | 22 +---
.../ContentObject/ContentObjectRenderer.php | 124 +++---------------
.../ContentObjectRendererTest.php | 69 ++++------
3 files changed, 45 insertions(+), 170 deletions(-)
diff --git a/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php b/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
index aac7aa1003c8..47a4831f1dd8 100644
--- a/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
+++ b/typo3/sysext/core/Classes/Database/Query/QueryBuilder.php
@@ -44,7 +44,6 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend;
use TYPO3\CMS\Extbase\Tests\Functional\Persistence\Generic\Storage\Typo3DbQueryParserTest;
-use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
/**
* Object-oriented approach to building SQL queries.
@@ -1217,8 +1216,7 @@ class QueryBuilder extends ConcreteQueryBuilder
/**
* Returns selected fields from internal query state.
*
- * @see ContentObjectRenderer::getQueryArray()
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
* @return string[]
*/
public function getSelect(): array
@@ -1230,10 +1228,9 @@ class QueryBuilder extends ConcreteQueryBuilder
/**
* Returns from tables from internal query state.
*
- * @see ContentObjectRenderer::getQueryArray()
* @see Typo3DbBackend::getObjectDataByQuery()
* @see Typo3DbBackend::getObjectCountByQuery()
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
* @return From[]
*/
public function getFrom()
@@ -1245,9 +1242,8 @@ class QueryBuilder extends ConcreteQueryBuilder
/**
* Returns where expressions from internal query state.
*
- * @see ContentObjectRenderer::getQueryArray()
* @see Typo3DbQueryParserTest
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
* @return CompositeExpression|string|null
*/
public function getWhere(): CompositeExpression|string|null
@@ -1259,8 +1255,7 @@ class QueryBuilder extends ConcreteQueryBuilder
/**
* Returns having expressions from internal query state.
*
- * @see ContentObjectRenderer::getQueryArray()
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
* @return CompositeExpression|string|null
*/
public function getHaving(): CompositeExpression|string|null
@@ -1273,10 +1268,9 @@ class QueryBuilder extends ConcreteQueryBuilder
* Returns order-by definitions from internal query state.
*
* @return string[]
- * @see ContentObjectRenderer::getQueryArray()
* @see RelationHandler::readForeignField()
* @see Typo3DbQueryParserTest
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
*/
public function getOrderBy(): array
{
@@ -1287,10 +1281,9 @@ class QueryBuilder extends ConcreteQueryBuilder
/**
* Returns selected group-by definitions from internal query state.
*
- * @see ContentObjectRenderer::getQueryArray()
* @see Typo3DbQueryParserTest
* @return string[]
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
*/
public function getGroupBy(): array
{
@@ -1301,9 +1294,8 @@ class QueryBuilder extends ConcreteQueryBuilder
/**
* Returns the list of joins, indexed by `from-alias` from the internal query state.
*
- * @see ContentObjectRenderer::getQueryArray()
* @return array<string, Join[]>
- * @internal only, used for Extbase internal handling, ContentObject Query handling and core tests. Don't use it.
+ * @internal only, used for Extbase internal handling and core tests. Don't use it.
*/
public function getJoin(): array
{
diff --git a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
index e0f5d7127531..4b5b16fc5279 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
@@ -17,7 +17,6 @@ namespace TYPO3\CMS\Frontend\ContentObject;
use Doctrine\DBAL\Exception as DBALException;
use Doctrine\DBAL\Query\From;
-use Doctrine\DBAL\Query\Join;
use Doctrine\DBAL\Result;
use Psr\Container\ContainerInterface;
use Psr\EventDispatcher\EventDispatcherInterface;
@@ -30,6 +29,7 @@ use TYPO3\CMS\Core\Context\Context;
use TYPO3\CMS\Core\Context\LanguageAspect;
use TYPO3\CMS\Core\Core\Environment;
use TYPO3\CMS\Core\Crypto\HashService;
+use TYPO3\CMS\Core\Database\Connection;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
use TYPO3\CMS\Core\Database\Query\QueryBuilder;
@@ -4688,8 +4688,8 @@ class ContentObjectRenderer implements LoggerAwareInterface
*/
public function exec_getQuery($table, $conf)
{
- $statement = $this->getQuery($table, $conf);
$connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
+ $statement = $this->getQuery($connection, $table, $conf);
return $connection->executeQuery($statement);
}
@@ -4729,22 +4729,21 @@ class ContentObjectRenderer implements LoggerAwareInterface
}
/**
- * Creates and returns a SELECT query for records from $table and with conditions based on the configuration in the $conf array
- * Implements the "select" function in TypoScript
+ * Creates and returns a SELECT query for records from $table and with conditions
+ * based on the configuration in the $conf array.
+ * Implements the "select" function in TypoScript.
*
* @param string $table See ->exec_getQuery()
* @param array $conf See ->exec_getQuery()
- * @param bool $returnQueryArray If set, the function will return the query not as a string but array with the various parts. RECOMMENDED!
- * @return mixed A SELECT query if $returnQueryArray is FALSE, otherwise the SELECT query in an array as parts.
+ * @return string A SELECT query
* @throws \RuntimeException
* @throws \InvalidArgumentException
* @internal
* @see numRows()
*/
- public function getQuery($table, $conf, $returnQueryArray = false)
+ public function getQuery(Connection $connection, string $table, array $conf): string
{
// Resolve stdWrap in these properties first
- $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
$properties = [
'pidInList',
'uidInList',
@@ -4777,7 +4776,7 @@ class ContentObjectRenderer implements LoggerAwareInterface
}
}
// Handle PDO-style named parameter markers first
- $queryMarkers = $this->getQueryMarkers($table, $conf);
+ $queryMarkers = $this->getQueryMarkers($connection, $conf);
// Replace the markers in the non-stdWrap properties
foreach ($queryMarkers as $marker => $markerValue) {
$properties = [
@@ -4819,7 +4818,7 @@ class ContentObjectRenderer implements LoggerAwareInterface
$conf['pidInList'] = 'this';
}
- $queryParts = $this->getQueryConstraints($table, $conf);
+ $queryParts = $this->getQueryConstraints($connection, $table, $conf);
$queryBuilder = $connection->createQueryBuilder();
// @todo Check against getQueryConstraints, can probably use FrontendRestrictions
@@ -4843,14 +4842,14 @@ class ContentObjectRenderer implements LoggerAwareInterface
// Fields:
if ($conf['selectFields'] ?? false) {
- $queryBuilder->selectLiteral($this->sanitizeSelectPart($conf['selectFields'], $table));
+ $queryBuilder->selectLiteral($this->sanitizeSelectPart($connection, $conf['selectFields'], $table));
}
// Setting LIMIT:
if (($conf['max'] ?? false) || ($conf['begin'] ?? false)) {
// Finding the total number of records, if used:
if (str_contains(strtolower(($conf['begin'] ?? '') . ($conf['max'] ?? '')), 'total')) {
- $countQueryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
+ $countQueryBuilder = $connection->createQueryBuilder();
$countQueryBuilder->getRestrictions()->removeAll();
$countQueryBuilder->count('*')
->from($table)
@@ -4920,92 +4919,9 @@ class ContentObjectRenderer implements LoggerAwareInterface
// @todo for exec_Query / getQuery it's the best we can do.
$query = str_replace('###' . $marker . '###', $markerValue, $query);
}
-
- return $returnQueryArray ? $this->getQueryArray($queryBuilder) : $query;
- }
-
- /**
- * Helper to transform a QueryBuilder object into a queryParts array that can be used
- * with exec_SELECT_queryArray
- *
- * @return array
- * @throws \RuntimeException
- */
- protected function getQueryArray(QueryBuilder $queryBuilder)
- {
- $fromClauses = [];
- $knownAliases = [];
- $queryParts = [];
- $fromParts = $queryBuilder->getFrom();
- $joinParts = $queryBuilder->getJoin();
-
- // Loop through all FROM clauses
- foreach ($fromParts as $from) {
- if ($from->alias === null) {
- $tableSql = $from->table;
- $tableReference = $from->table;
- } else {
- $tableSql = $from->table . ' ' . $from->alias;
- $tableReference = $from->alias;
- }
-
- $knownAliases[$tableReference] = true;
-
- $fromClauses[$tableReference] = $tableSql . $this->getQueryArrayJoinHelper(
- $tableReference,
- $joinParts,
- $knownAliases
- );
- }
-
- $queryParts['SELECT'] = implode(', ', $queryBuilder->getSelect());
- $queryParts['FROM'] = implode(', ', $fromClauses);
- $queryParts['WHERE'] = (string)$queryBuilder->getWhere() ?: '';
- $queryParts['GROUPBY'] = implode(', ', $queryBuilder->getGroupBy());
- $queryParts['ORDERBY'] = implode(', ', $queryBuilder->getOrderBy());
- if ($queryBuilder->getFirstResult() > 0) {
- $queryParts['LIMIT'] = $queryBuilder->getFirstResult() . ',' . $queryBuilder->getMaxResults();
- } elseif ($queryBuilder->getMaxResults() > 0) {
- $queryParts['LIMIT'] = $queryBuilder->getMaxResults();
- }
-
- return $queryParts;
+ return $query;
}
- /**
- * Helper to transform the QueryBuilder join part into a SQL fragment.
- *
- * @param array<string, Join[]> $joinParts
- * @param array<string, bool> $knownAliases
- *
- * @throws \RuntimeException
- */
- protected function getQueryArrayJoinHelper(string $fromAlias, array $joinParts, array &$knownAliases): string
- {
- $sql = '';
-
- if (isset($joinParts[$fromAlias])) {
- foreach ($joinParts[$fromAlias] as $join) {
- /** @var Join $join */
- if (array_key_exists($join->alias, $knownAliases)) {
- throw new \RuntimeException(
- 'Non unique join alias: "' . $join->alias . '" found.',
- 1472748872
- );
- }
- $sql .= ' ' . strtoupper($join->type)
- . ' JOIN ' . $join->table . ' ' . $join->alias
- . ' ON ' . ((string)$join->condition);
- $knownAliases[$join->alias] = true;
- }
-
- foreach ($joinParts[$fromAlias] as $join) {
- $sql .= $this->getQueryArrayJoinHelper($join->alias, $joinParts, $knownAliases);
- }
- }
-
- return $sql;
- }
/**
* Helper function for getQuery(), creating the WHERE clause of the SELECT query
*
@@ -5015,10 +4931,9 @@ class ContentObjectRenderer implements LoggerAwareInterface
* @throws \InvalidArgumentException
* @see getQuery()
*/
- protected function getQueryConstraints(string $table, array $conf): array
+ protected function getQueryConstraints(Connection $connection, string $table, array $conf): array
{
- // Init:
- $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
+ $queryBuilder = $connection->createQueryBuilder();
$expressionBuilder = $queryBuilder->expr();
$request = $this->getRequest();
$contentPid = $request->getAttribute('frontend.page.information')->getContentFromPid();
@@ -5205,16 +5120,12 @@ class ContentObjectRenderer implements LoggerAwareInterface
* This functions checks if the necessary fields are part of the select
* and adds them if necessary.
*
- * @param string $selectPart Select part
- * @param string $table Table to select from
* @return string Sanitized select part
* @internal
* @see getQuery
*/
- protected function sanitizeSelectPart($selectPart, $table)
+ protected function sanitizeSelectPart(Connection $connection, string $selectPart, string $table)
{
- $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
-
// Pattern matching parts
$matchStart = '/(^\\s*|,\\s*|' . $table . '\\.)';
$matchEnd = '(\\s*,|\\s*$)/';
@@ -5280,19 +5191,16 @@ class ContentObjectRenderer implements LoggerAwareInterface
* Builds list of marker values for handling PDO-like parameter markers in select parts.
* Marker values support stdWrap functionality thus allowing a way to use stdWrap functionality in various properties of 'select' AND prevents SQL-injection problems by quoting and escaping of numeric values, strings, NULL values and comma separated lists.
*
- * @param string $table Table to select records from
* @param array $conf Select part of CONTENT definition
* @return array List of values to replace markers with
* @internal
* @see getQuery()
*/
- public function getQueryMarkers($table, $conf)
+ public function getQueryMarkers(Connection $connection, $conf)
{
if (!isset($conf['markers.']) || !is_array($conf['markers.'])) {
return [];
}
- // Parse markers and prepare their values
- $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
$markerValues = [];
foreach ($conf['markers.'] as $dottedMarker => $dummy) {
$marker = rtrim($dottedMarker, '.');
diff --git a/typo3/sysext/frontend/Tests/Functional/ContentObject/ContentObjectRendererTest.php b/typo3/sysext/frontend/Tests/Functional/ContentObject/ContentObjectRendererTest.php
index 55297841507b..22c1db10780b 100644
--- a/typo3/sysext/frontend/Tests/Functional/ContentObject/ContentObjectRendererTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/ContentObject/ContentObjectRendererTest.php
@@ -87,36 +87,28 @@ final class ContentObjectRendererTest extends FunctionalTestCase
'testing empty conf' => [
'tt_content',
[],
- [
- 'SELECT' => '*',
- ],
+ '*',
],
'testing #17284: adding uid/pid for workspaces' => [
'tt_content',
[
'selectFields' => 'header,bodytext',
],
- [
- 'SELECT' => 'header,bodytext, [tt_content].[uid] AS [uid], [tt_content].[pid] AS [pid], [tt_content].[t3ver_state] AS [t3ver_state]',
- ],
+ 'header,bodytext, [tt_content].[uid] AS [uid], [tt_content].[pid] AS [pid], [tt_content].[t3ver_state] AS [t3ver_state]',
],
'testing #17284: no need to add' => [
'tt_content',
[
'selectFields' => 'tt_content.*',
],
- [
- 'SELECT' => 'tt_content.*',
- ],
+ 'tt_content.*',
],
'testing #17284: no need to add #2' => [
'tt_content',
[
'selectFields' => '*',
],
- [
- 'SELECT' => '*',
- ],
+ '*',
],
'testing #29783: joined tables, prefix tablename' => [
'tt_content',
@@ -124,63 +116,49 @@ final class ContentObjectRendererTest extends FunctionalTestCase
'selectFields' => 'tt_content.header,be_users.username',
'join' => 'be_users ON tt_content.cruser_id = be_users.uid',
],
- [
- 'SELECT' => 'tt_content.header,be_users.username, [tt_content].[uid] AS [uid], [tt_content].[pid] AS [pid], [tt_content].[t3ver_state] AS [t3ver_state]',
- ],
+ 'tt_content.header,be_users.username, [tt_content].[uid] AS [uid], [tt_content].[pid] AS [pid], [tt_content].[t3ver_state] AS [t3ver_state]',
],
'testing #34152: single count(*), add nothing' => [
'tt_content',
[
'selectFields' => 'count(*)',
],
- [
- 'SELECT' => 'count(*)',
- ],
+ 'count(*)',
],
'testing #34152: single max(crdate), add nothing' => [
'tt_content',
[
'selectFields' => 'max(crdate)',
],
- [
- 'SELECT' => 'max(crdate)',
- ],
+ 'max(crdate)',
],
'testing #34152: single min(crdate), add nothing' => [
'tt_content',
[
'selectFields' => 'min(crdate)',
],
- [
- 'SELECT' => 'min(crdate)',
- ],
+ 'min(crdate)',
],
'testing #34152: single sum(is_siteroot), add nothing' => [
'tt_content',
[
'selectFields' => 'sum(is_siteroot)',
],
- [
- 'SELECT' => 'sum(is_siteroot)',
- ],
+ 'sum(is_siteroot)',
],
'testing #34152: single avg(crdate), add nothing' => [
'tt_content',
[
'selectFields' => 'avg(crdate)',
],
- [
- 'SELECT' => 'avg(crdate)',
- ],
+ 'avg(crdate)',
],
'single distinct, add nothing' => [
'tt_content',
[
'selectFields' => 'DISTINCT crdate',
],
- [
- 'SELECT' => 'DISTINCT crdate',
- ],
+ 'DISTINCT crdate',
],
'testing #96321: pidInList=root does not raise PHP 8 warning' => [
'tt_content',
@@ -189,9 +167,7 @@ final class ContentObjectRendererTest extends FunctionalTestCase
'recursive' => '5',
'pidInList' => 'root',
],
- [
- 'SELECT' => '*',
- ],
+ '*',
],
];
}
@@ -201,7 +177,7 @@ final class ContentObjectRendererTest extends FunctionalTestCase
*/
#[DataProvider('getQueryDataProvider')]
#[Test]
- public function getQuery(string $table, array $conf, array $expected): void
+ public function getQuery(string $table, array $conf, string $expected): void
{
$GLOBALS['TCA'] = [
'pages' => [
@@ -230,18 +206,17 @@ final class ContentObjectRendererTest extends FunctionalTestCase
$request = $request->withAttribute('frontend.page.information', $pageInformation);
$subject->setRequest($request);
- $result = $subject->getQuery($table, $conf, true);
+ $connection = (new ConnectionPool())->getConnectionForTable('tt_content');
+ $result = $subject->getQuery($connection, $table, $conf);
- $databasePlatform = (new ConnectionPool())->getConnectionForTable('tt_content')->getDatabasePlatform();
+ $databasePlatform = $connection->getDatabasePlatform();
$identifierQuoteCharacter = (new PlatformHelper())->getIdentifierQuoteCharacter($databasePlatform);
- foreach ($expected as $field => $value) {
- // Replace the TYPO3 quote character with the actual quote character for the DBMS,
- if ($field === 'SELECT') {
- $quoteChar = $identifierQuoteCharacter;
- $value = str_replace(['[', ']'], [$quoteChar, $quoteChar], $value);
- }
- self::assertEquals($value, $result[$field]);
- }
+ // strip select * from part between SELECT and FROM
+ $selectValue = preg_replace('/SELECT (.*) FROM.*/', '$1', $result);
+ // Replace the TYPO3 quote character with the actual quote character for the DBMS
+ $quoteChar = $identifierQuoteCharacter;
+ $expected = str_replace(['[', ']'], [$quoteChar, $quoteChar], $expected);
+ self::assertEquals($expected, $selectValue);
}
#[Test]
--
GitLab