From 9d914dcb57a51fe2953f32241cd1423b30ed456a Mon Sep 17 00:00:00 2001
From: Sascha Nowak <sascha.nowak@netlogix.de>
Date: Sat, 17 Feb 2024 10:46:28 +0100
Subject: [PATCH] [BUGFIX] Avoid broken treelist cache
The cache_treelist handling in PageRepository is broken
for a long time already: Entries are get() with an expiry
time that is not properly set(), so get() is never
successful.
There is a second bug in getDescendantPageIdsRecursive()
cache handling: get() is done without BE user restriction,
but set() is only done when no BE user is logged in. This
way, non-BE user state could swap into BE user state, *if*
the cache would work.
The patch avoids reading and writing the cache instead
of fixing cache access: We can not risk bugs in v12 if
the cache would suddenly start working but has side effects:
Issues like that would be hard to find, hard to report,
and have severe impact on FE rendering.
Note typical use case of the method are the "menu" content
elements, plus the extbase 'starting point' logic. So its
not the menu rendering in general - we would otherwise have
most likely found the broken cache much earlier.
The patch effectively increases performance since the
get() and set() queries are gone.
Resolves: #103139
Releases: main, 12.4
Change-Id: Icc8d13b583f77d6ffc186d391d46d6830ee50889
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83055
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
---
.../Domain/Repository/PageRepository.php | 80 +++----------------
1 file changed, 13 insertions(+), 67 deletions(-)
diff --git a/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php b/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
index 284ea3e20ba1..11db61ceb5f0 100644
--- a/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
+++ b/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
@@ -1891,23 +1891,21 @@ class PageRepository implements LoggerAwareInterface
* Generates a list of Page IDs from $startPageId. List does not include $startPageId itself.
* Only works on default language level.
*
- * The only pages WHICH PREVENTS DESCENDING in a branch are
- * - deleted pages,
- * - pages in a recycler (doktype = 255) or of the Backend User Section (doktype = 6) type
- * - pages that have the extendToSubpages set, WHERE starttime, endtime, hidden or fe_group
- * would hide the pages.
+ * Pages that prevent looking for further subpages:
+ * - deleted pages
+ * - pages of the Backend User Section (doktype = 6) and recycler (doktype = 255) type
+ * - pages that have the extendToSubpages set, where starttime, endtime, hidden or fe_group
+ * would hide the pages
*
- * Apart from that, pages with enable-fields excluding them, will also be
- * removed.
+ * Apart from that, pages with enable-fields excluding them, will also be removed.
*
- * Mount Pages are also descended but notice that these ID numbers are not
- * useful for links unless the correct MPvar is set.
+ * Mount Pages are descended, but note these ID numbers are not useful for links unless the correct MPvar is set.
*
* @param int $startPageId The id of the start page from which point in the page tree to descend.
- * @param int $depth The number of levels to descend. If you want to descend infinitely, just set this to 100 or so. Should be at least "1" since zero will just make the function return (no descend...)
- * @param int $begin Is an optional integer that determines at which level in the tree to start collecting uid's. Zero means 'start right away', 1 = 'next level and out'
- * @param array $excludePageIds avoid collecting these pages and their possible subpages
- * @param bool $bypassEnableFieldsCheck if true, then enableFields and other checks are not evaluated
+ * @param int $depth Maximum recursion depth. Use 100 or so to descend "infinitely". Stops when 0 is reached.
+ * @param int $begin An optional integer the level in the tree to start collecting. Zero means 'start right away', 1 = 'next level and out'
+ * @param array $excludePageIds Avoid collecting these pages and their possible subpages
+ * @param bool $bypassEnableFieldsCheck If true, then enableFields and other checks are not evaluated
* @return int[] Returns the list of Page IDs
*/
public function getDescendantPageIdsRecursive(int $startPageId, int $depth, int $begin = 0, array $excludePageIds = [], bool $bypassEnableFieldsCheck = false): array
@@ -1915,43 +1913,8 @@ class PageRepository implements LoggerAwareInterface
if (!$startPageId) {
return [];
}
-
- // Check the cache
- $parameters = [
- $startPageId,
- $depth,
- $begin,
- $excludePageIds,
- $bypassEnableFieldsCheck,
- $this->context->getPropertyFromAspect('frontend.user', 'groupIds', [0, -1]),
- ];
- $cacheIdentifier = md5(serialize($parameters));
- $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
- ->getQueryBuilderForTable('cache_treelist');
- $cacheEntry = $queryBuilder->select('treelist')
- ->from('cache_treelist')
- ->where(
- $queryBuilder->expr()->eq(
- 'md5hash',
- $queryBuilder->createNamedParameter($cacheIdentifier)
- ),
- $queryBuilder->expr()->gt(
- 'expires',
- $queryBuilder->createNamedParameter($GLOBALS['EXEC_TIME'], Connection::PARAM_INT)
- )
- )
- ->setMaxResults(1)
- ->executeQuery()
- ->fetchOne();
-
- // Cache hit
- if (!empty($cacheEntry)) {
- return GeneralUtility::intExplode(',', $cacheEntry);
- }
-
- // Check if the page actually exists
if (!$this->getRawRecord('pages', $startPageId, 'uid')) {
- // Return blank if the start page was NOT found at all!
+ // Start page does not exist
return [];
}
// Find mount point if any
@@ -1959,32 +1922,15 @@ class PageRepository implements LoggerAwareInterface
$includePageId = false;
if (is_array($mount_info)) {
$startPageId = (int)$mount_info['mount_pid'];
- // In Overlay mode, use the mounted page uid as added ID!
+ // In overlay mode, use the mounted page uid
if ($mount_info['overlay']) {
$includePageId = true;
}
}
-
$descendantPageIds = $this->getSubpagesRecursive($startPageId, $depth, $begin, $excludePageIds, $bypassEnableFieldsCheck);
if ($includePageId) {
$descendantPageIds = array_merge([$startPageId], $descendantPageIds);
}
- // Only add to cache if not logged into TYPO3 Backend
- if (!$this->context->getPropertyFromAspect('backend.user', 'isLoggedIn', false)) {
- $cacheEntry = [
- 'md5hash' => $cacheIdentifier,
- 'pid' => $startPageId,
- 'treelist' => implode(',', $descendantPageIds),
- 'tstamp' => $GLOBALS['EXEC_TIME'],
- ];
- $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('cache_treelist');
- try {
- $connection->transactional(static function ($connection) use ($cacheEntry) {
- $connection->insert('cache_treelist', $cacheEntry);
- });
- } catch (\Throwable $e) {
- }
- }
return $descendantPageIds;
}
--
GitLab