Commit ad385665 authored by Benni Mack's avatar Benni Mack
Browse files

[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/+/72772


Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
parent e9b43754
......@@ -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:
......
......@@ -1245,41 +1245,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'];
......@@ -1385,7 +1352,7 @@ abstract class AbstractMenuContentObject
],
[
rawurlencode($LD['totalURL']),
$page['_SHORTCUT_PAGE_UID'] ?? $page['uid'],
$page['uid'],
],
$this->mconf['showAccessRestrictedPages.']['addParams']
);
......
......@@ -121,36 +121,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 = [];
......@@ -203,6 +186,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)) {
......@@ -314,6 +305,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;
......@@ -332,6 +324,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'];
......@@ -339,6 +333,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
*
......
......@@ -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'],
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment