From d5c7a96384b7c7cb489fc7ede707773382290fca Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Fri, 1 Oct 2021 14:38:50 +0200
Subject: [PATCH] [BUGFIX] Resolve shortcuts in typolink directly
Currently, HMENU generates the direct destination for
a shortcut URL, but typolink does not. This is an
inconsistency, and since HMENU is actually using typolink
now as well (in 4.x this was different), the resolving
of shortcuts can now be moved into the typolink/link building
functionality.
Page Shortcuts are now resolved (recursively!) at a single point for
all links, except for RANDOM_SUBPAGE variants, as this is sorted
out during runtime when hitting the RANDOM_SUBPAGE shortcut page.
Resolves: #80113
Resolves: #95947
Releases: main, 11.5
Change-Id: Ib6204ff1b0ecb699ec87f5154dc18c974676e65c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/71402
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Benni Mack <benni@typo3.org>
---
.../Domain/Repository/PageRepository.php | 8 +-
.../Menu/AbstractMenuContentObject.php | 37 +--------
.../Classes/Typolink/PageLinkBuilder.php | 79 ++++++++++++-------
.../SiteHandling/SlugLinkGeneratorTest.php | 18 ++---
4 files changed, 68 insertions(+), 74 deletions(-)
diff --git a/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php b/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
index 1033fea54b34..277d37092b76 100644
--- a/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
+++ b/typo3/sysext/core/Classes/Domain/Repository/PageRepository.php
@@ -966,16 +966,20 @@ class PageRepository implements LoggerAwareInterface
* @param int $iteration Safety feature which makes sure that the function is calling itself recursively max 20 times (since this function can find shortcuts to other shortcuts to other shortcuts...)
* @param array $pageLog An array filled with previous page uids tested by the function - new page uids are evaluated against this to avoid going in circles.
* @param bool $disableGroupCheck If true, the group check is disabled when fetching the target page (needed e.g. for menu generation)
+ * @param bool $resolveRandomPageShortcuts If true (default) this will also resolve shortcut to random subpages. In case of linking from a page to a shortcut page, we do not want to cache the "random" logic.
*
* @throws \RuntimeException
* @throws ShortcutTargetPageNotFoundException
- * @return mixed Returns the page record of the page that the shortcut pointed to.
+ * @return mixed Returns the page record of the page that the shortcut pointed to. If $resolveRandomPageShortcuts = false, and the shortcut page is configured to point to a random shortcut then an empty array is returned
* @internal
* @see getPageAndRootline()
*/
- public function getPageShortcut($shortcutFieldValue, $shortcutMode, $thisUid, $iteration = 20, $pageLog = [], $disableGroupCheck = false)
+ public function getPageShortcut($shortcutFieldValue, $shortcutMode, $thisUid, $iteration = 20, $pageLog = [], $disableGroupCheck = false, bool $resolveRandomPageShortcuts = true)
{
$idArray = GeneralUtility::intExplode(',', $shortcutFieldValue);
+ if ($resolveRandomPageShortcuts === false && (int)$shortcutMode === self::SHORTCUT_MODE_RANDOM_SUBPAGE) {
+ return [];
+ }
// Find $page record depending on shortcut mode:
switch ($shortcutMode) {
case self::SHORTCUT_MODE_FIRST_SUBPAGE:
diff --git a/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php b/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
index 4fd0d808562c..07cbca8e66b5 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php
@@ -1242,41 +1242,8 @@ abstract class AbstractMenuContentObject
$LD['target'] = $this->menuArr[$key]['target'];
}
- // Override url if current page is a shortcut
- $shortcut = null;
- if ((int)($this->menuArr[$key]['doktype'] ?? 0) === PageRepository::DOKTYPE_SHORTCUT && (int)$this->menuArr[$key]['shortcut_mode'] !== PageRepository::SHORTCUT_MODE_RANDOM_SUBPAGE) {
- $menuItem = $this->menuArr[$key];
- try {
- $shortcut = $tsfe->sys_page->getPageShortcut(
- $menuItem['shortcut'],
- $menuItem['shortcut_mode'],
- $menuItem['uid'],
- 20,
- [],
- true
- );
- } catch (\Exception $ex) {
- }
- if (!is_array($shortcut)) {
- $runtimeCache->set($cacheId, []);
- return [];
- }
- // Only setting url, not target
- $LD['totalURL'] = $this->parent_cObj->typoLink_URL([
- 'parameter' => $shortcut['uid'],
- 'language' => $menuItem['_PAGES_OVERLAY_REQUESTEDLANGUAGE'] ?? 'current',
- 'additionalParams' => $addParams . $this->I['val']['additionalParams'] . ($menuItem['_ADD_GETVARS'] ?? ''),
- 'linkAccessRestrictedPages' => !empty($this->mconf['showAccessRestrictedPages']),
- ]);
- }
- if ($shortcut) {
- $pageData = $shortcut;
- $pageData['_SHORTCUT_PAGE_UID'] = $this->menuArr[$key]['uid'];
- } else {
- $pageData = $this->menuArr[$key];
- }
// Manipulation in case of access restricted pages:
- $this->changeLinksForAccessRestrictedPages($LD, $pageData, $mainTarget, $typeOverride);
+ $this->changeLinksForAccessRestrictedPages($LD, $this->menuArr[$key], $mainTarget, $typeOverride);
// Overriding URL / Target if set to do so:
if ($this->menuArr[$key]['_OVERRIDE_HREF'] ?? false) {
$LD['totalURL'] = $this->menuArr[$key]['_OVERRIDE_HREF'];
@@ -1343,7 +1310,7 @@ abstract class AbstractMenuContentObject
],
[
rawurlencode($LD['totalURL']),
- $page['_SHORTCUT_PAGE_UID'] ?? $page['uid'],
+ $page['uid'],
],
$this->mconf['showAccessRestrictedPages.']['addParams']
);
diff --git a/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
index b26c17e19157..62cae99795b6 100644
--- a/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
+++ b/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
@@ -117,36 +117,19 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
if ($addQueryParams === '&' || ($addQueryParams[0] ?? '') !== '&') {
$addQueryParams = '';
}
- // Mount pages are always local and never link to another domain
- if (!empty($MPvarAcc)) {
- // Add "&MP" var:
- $addQueryParams .= '&MP=' . rawurlencode(implode(',', $MPvarAcc));
- } elseif (!str_contains($addQueryParams, '&MP=')) {
- // We do not come here if additionalParams had '&MP='. This happens when typoLink is called from
- // menu. Mount points always work in the content of the current domain and we must not change
- // domain if MP variables exist.
- // If we link across domains and page is free type shortcut, we must resolve the shortcut first!
- if ((int)($page['doktype'] ?? 0) === PageRepository::DOKTYPE_SHORTCUT
- && (int)($page['shortcut_mode'] ?? 0) === PageRepository::SHORTCUT_MODE_NONE
- ) {
- // Save in case of broken destination or endless loop
- $page2 = $page;
- // Same as in RealURL, seems enough
- $maxLoopCount = 20;
- while ($maxLoopCount
- && is_array($page)
- && (int)($page['doktype'] ?? 0) === PageRepository::DOKTYPE_SHORTCUT
- && (int)($page['shortcut_mode'] ?? 0) === PageRepository::SHORTCUT_MODE_NONE
- ) {
- $page = $tsfe->sys_page->getPage($page['shortcut'], $disableGroupAccessCheck);
- $maxLoopCount--;
- }
- if (empty($page) || $maxLoopCount === 0) {
- // We revert if shortcut is broken or maximum number of loops is exceeded (indicates endless loop)
- $page = $page2;
- }
+ // Mount pages are always local and never link to another domain,
+ $addMountPointParameters = !empty($MPvarAcc);
+ // Add "&MP" var, only if the original page was NOT a shortcut to another domain
+ if ($addMountPointParameters && !empty($page['_SHORTCUT_PAGE_UID'])) {
+ $siteOfTargetPage = GeneralUtility::makeInstance(SiteFinder::class)->getSiteByPageId((int)$page['_SHORTCUT_PAGE_UID']);
+ $currentSite = $this->getCurrentSite();
+ if ($siteOfTargetPage !== $currentSite) {
+ $addMountPointParameters = false;
}
}
+ if ($addMountPointParameters) {
+ $addQueryParams .= '&MP=' . rawurlencode(implode(',', $MPvarAcc));
+ }
// get config.linkVars and prepend them before the actual GET parameters
$queryParameters = [];
@@ -199,6 +182,14 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
$context->setAspect('language', $languageAspect);
$pageRepository = GeneralUtility::makeInstance(PageRepository::class, $context);
$page = $pageRepository->getPageOverlay($page);
+ // Check if the translated page is a shortcut, but the default page wasn't a shortcut, so this is
+ // resolved as well, see ScenarioDTest in functional tests.
+ // Currently not supported: When this is the case (only a translated page is a shortcut),
+ // but the page links to a different site.
+ $shortcutPage = $this->resolveShortcutPage($page, $pageRepository, $disableGroupAccessCheck);
+ if (!empty($shortcutPage)) {
+ $page = $shortcutPage;
+ }
}
// Check if the target page can be access depending on l18n_cfg
if (!$tsfe->sys_page->isPageSuitableForLanguage($page, $languageAspect)) {
@@ -310,6 +301,7 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
if (empty($page) || !is_array($page)) {
return [];
}
+ $page = $this->resolveShortcutPage($page, $pageRepository, $disableGroupAccessCheck);
$languageField = $GLOBALS['TCA']['pages']['ctrl']['languageField'] ?? null;
$languageParentField = $GLOBALS['TCA']['pages']['ctrl']['transOrigPointerField'] ?? null;
@@ -328,6 +320,8 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
if (empty($languageParentPage)) {
return $page;
}
+ // Check for the shortcut of the default-language page
+ $languageParentPage = $this->resolveShortcutPage($languageParentPage, $pageRepository, $disableGroupAccessCheck);
// Set the "pageuid" to the default-language page ID.
$linkDetails['pageuid'] = (int)$languageParentPage['uid'];
@@ -335,6 +329,35 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
return $languageParentPage;
}
+ /**
+ * Check if page is a shortcut, then resolve the target page directly
+ */
+ protected function resolveShortcutPage(array $page, PageRepository $pageRepository, bool $disableGroupAccessCheck): array
+ {
+ if ((int)($page['doktype'] ?? 0) !== PageRepository::DOKTYPE_SHORTCUT) {
+ return $page;
+ }
+ $shortcutMode = (int)($page['shortcut_mode'] ?? PageRepository::SHORTCUT_MODE_NONE);
+ try {
+ $shortcut = $pageRepository->getPageShortcut(
+ $page['shortcut'] ?? '',
+ $shortcutMode,
+ $page['uid'],
+ 20,
+ [],
+ $disableGroupAccessCheck,
+ false
+ );
+ if (!empty($shortcut)) {
+ $page = $shortcut;
+ $page['_SHORTCUT_PAGE_UID'] = $page['uid'];
+ }
+ } catch (\Exception $e) {
+ return $page;
+ }
+ return $page;
+ }
+
/**
* Fetches the requested language of a site that the link should be built for
*
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
index 05b2bc4d0b90..c58bc5c223d9 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php
@@ -125,7 +125,7 @@ class SlugLinkGeneratorTest extends AbstractTestCase
{
$instructions = [
// acme.com -> acme.com (same site)
- ['https://acme.us/', 1100, 1000, '/'],
+ ['https://acme.us/', 1100, 1000, '/welcome'], // shortcut page is resolved directly
['https://acme.us/', 1100, 1100, '/welcome'],
['https://acme.us/', 1100, 1200, '/features'],
['https://acme.us/', 1100, 1210, '/features/frontend-editing/'],
@@ -134,12 +134,12 @@ class SlugLinkGeneratorTest extends AbstractTestCase
['https://acme.us/', 1100, 1300, 'https://products.acme.com/products'],
['https://acme.us/', 1100, 1310, 'https://products.acme.com/products/planets'],
// acme.com -> blog.acme.com (different site)
- ['https://acme.us/', 1100, 2000, 'https://blog.acme.com/'],
+ ['https://acme.us/', 1100, 2000, 'https://blog.acme.com/authors'], // recursive shortcut page is resolved directly
['https://acme.us/', 1100, 2100, 'https://blog.acme.com/authors'],
['https://acme.us/', 1100, 2110, 'https://blog.acme.com/john/john'],
['https://acme.us/', 1100, 2111, 'https://blog.acme.com/john/about-john'],
// blog.acme.com -> acme.com (different site)
- ['https://blog.acme.com/', 2100, 1000, 'https://acme.us/'],
+ ['https://blog.acme.com/', 2100, 1000, 'https://acme.us/welcome'], // shortcut page is resolved directly
['https://blog.acme.com/', 2100, 1100, 'https://acme.us/welcome'],
['https://blog.acme.com/', 2100, 1200, 'https://acme.us/features'],
['https://blog.acme.com/', 2100, 1210, 'https://acme.us/features/frontend-editing/'],
@@ -186,7 +186,7 @@ class SlugLinkGeneratorTest extends AbstractTestCase
{
$instructions = [
// acme.com -> acme.com (same site)
- ['https://acme.us/', [7100, 1700], 7110, 1000, '/'],
+ ['https://acme.us/', [7100, 1700], 7110, 1000, '/welcome'], // shortcut page is resolved directly
['https://acme.us/', [7100, 1700], 7110, 1100, '/welcome'],
['https://acme.us/', [7100, 1700], 7110, 1200, '/features'],
['https://acme.us/', [7100, 1700], 7110, 1210, '/features/frontend-editing/'],
@@ -195,12 +195,12 @@ class SlugLinkGeneratorTest extends AbstractTestCase
['https://acme.us/', [7100, 1700], 7110, 1300, 'https://products.acme.com/products'],
['https://acme.us/', [7100, 1700], 7110, 1310, 'https://products.acme.com/products/planets'],
// acme.com -> blog.acme.com (different site)
- ['https://acme.us/', [7100, 1700], 7110, 2000, 'https://blog.acme.com/'],
+ ['https://acme.us/', [7100, 1700], 7110, 2000, 'https://blog.acme.com/authors'], // shortcut page is resolved directly
['https://acme.us/', [7100, 1700], 7110, 2100, 'https://blog.acme.com/authors'],
['https://acme.us/', [7100, 1700], 7110, 2110, 'https://blog.acme.com/john/john'],
['https://acme.us/', [7100, 1700], 7110, 2111, 'https://blog.acme.com/john/about-john'],
// blog.acme.com -> acme.com (different site)
- ['https://blog.acme.com/', [7100, 2700], 7110, 1000, 'https://acme.us/'],
+ ['https://blog.acme.com/', [7100, 2700], 7110, 1000, 'https://acme.us/welcome'], // shortcut page is resolved directly
['https://blog.acme.com/', [7100, 2700], 7110, 1100, 'https://acme.us/welcome'],
['https://blog.acme.com/', [7100, 2700], 7110, 1200, 'https://acme.us/features'],
['https://blog.acme.com/', [7100, 2700], 7110, 1210, 'https://acme.us/features/frontend-editing/'],
@@ -334,7 +334,7 @@ class SlugLinkGeneratorTest extends AbstractTestCase
{
$instructions = [
// acme.com -> acme.com (same site)
- ['https://acme.us/', 1100, 1000, '/?testing%5Bvalue%5D=1&cHash=7d1f13fa91159dac7feb3c824936b39d'],
+ ['https://acme.us/', 1100, 1000, '/welcome?testing%5Bvalue%5D=1&cHash=f42b850e435f0cedd366f5db749fc1af'], // shortcut page is resolved directly
['https://acme.us/', 1100, 1100, '/welcome?testing%5Bvalue%5D=1&cHash=f42b850e435f0cedd366f5db749fc1af'],
['https://acme.us/', 1100, 1200, '/features?testing%5Bvalue%5D=1&cHash=784e11c50ea1a13fd7d969df4ec53ea3'],
['https://acme.us/', 1100, 1210, '/features/frontend-editing/?testing%5Bvalue%5D=1&cHash=ccb7067022b9835ebfd8f720722bc708'],
@@ -343,12 +343,12 @@ class SlugLinkGeneratorTest extends AbstractTestCase
['https://acme.us/', 1100, 1300, 'https://products.acme.com/products?testing%5Bvalue%5D=1&cHash=dbd6597d72ed5098cce3d03eac1eeefe'],
['https://acme.us/', 1100, 1310, 'https://products.acme.com/products/planets?testing%5Bvalue%5D=1&cHash=e64bfc7ab7dd6b70d161e4d556be9726'],
// acme.com -> blog.acme.com (different site)
- ['https://acme.us/', 1100, 2000, 'https://blog.acme.com/?testing%5Bvalue%5D=1&cHash=a14da633e46dba71640cb85226cd12c5'],
+ ['https://acme.us/', 1100, 2000, 'https://blog.acme.com/authors?testing%5Bvalue%5D=1&cHash=d23d74cb50383f8788a9930ec8ba679f'], // shortcut page is resolved directly
['https://acme.us/', 1100, 2100, 'https://blog.acme.com/authors?testing%5Bvalue%5D=1&cHash=d23d74cb50383f8788a9930ec8ba679f'],
['https://acme.us/', 1100, 2110, 'https://blog.acme.com/john/john?testing%5Bvalue%5D=1&cHash=bf25eea89f44a9a79dabdca98f38a432'],
['https://acme.us/', 1100, 2111, 'https://blog.acme.com/john/about-john?testing%5Bvalue%5D=1&cHash=42dbaeb9172b6b1ca23b49941e194db2'],
// blog.acme.com -> acme.com (different site)
- ['https://blog.acme.com/', 2100, 1000, 'https://acme.us/?testing%5Bvalue%5D=1&cHash=7d1f13fa91159dac7feb3c824936b39d'],
+ ['https://blog.acme.com/', 2100, 1000, 'https://acme.us/welcome?testing%5Bvalue%5D=1&cHash=f42b850e435f0cedd366f5db749fc1af'], // shortcut page is resolved directly
['https://blog.acme.com/', 2100, 1100, 'https://acme.us/welcome?testing%5Bvalue%5D=1&cHash=f42b850e435f0cedd366f5db749fc1af'],
['https://blog.acme.com/', 2100, 1200, 'https://acme.us/features?testing%5Bvalue%5D=1&cHash=784e11c50ea1a13fd7d969df4ec53ea3'],
['https://blog.acme.com/', 2100, 1210, 'https://acme.us/features/frontend-editing/?testing%5Bvalue%5D=1&cHash=ccb7067022b9835ebfd8f720722bc708'],
--
GitLab