Bug #95882: Redirect handling error in TypoScript conditions - TYPO3 Core - TYPO3 Forge

Actions

Copy link

Bug #95882

closed

Redirect handling error in TypoScript conditions

Added by Philip Masser about 3 years ago. Updated about 2 years ago.

Status:

Closed

Priority:

Should have

Assignee:

Stefan Bürk

Category:

Site Handling, Site Sets & Routing

Target version:

Start date:

2021-11-05

Due date:

% Done:

100%

Estimated time:

TYPO3 Version:

PHP Version:

7.2

Tags:

Complexity:

Is Regression:

Sprint Focus:

Description

I have a Typo3 10.4.12 with some conditions in global TS templates, regarding Site language (based onlanguage or fallback languages).

When adding a Redirect in the Redirect Backend Module, and then entering the Redirect URL into a browser, I get a Typo3 Exception:
#1536950931 TYPO3\CMS\Core\Configuration\TypoScript\Exception\InvalidTypoScriptConditionException Invalid expression in condition: [siteLanguage("languageId") == 2 || 2 in siteLanguage("fallbackLanguageIds")]

My analysis shows this:
- The RedirectHandler->process method determines that a redirect matches and tries to find the target URL. This causes TS evaluation
- The siteLanguage condition gets the site language of the request argument (cf. Typo3ConditionFunctionProvider:159: $requestWrapper = $arguments['request'];).
- However, this request has no Language, thus getSiteLanguage returns null (in fact, it has no arguments at all)
- This causes the in check to throw an exception (2 in null)
- The request in the evaluation is (indirectely) initialized in the TypoScriptFrontendController->bootFrontendController, via $request = $request ?? $GLOBALS['TYPO3_REQUEST'] ?? ServerRequestFactory::fromGlobals();
- $GLOBALS['TYPO3_REQUEST'] is not set, so a new request is created from globals. This has no arguments set, like the language

A preprocessed request is already available in the RedirectHandler, but not yet set in the $GLOBALS.
A possible solution is to set this request when a redirect matches:

// In TYPO3\CMS\Redirects\Http\Middleware\RedirectHandler->process
    // [...]
    // If the matched redirect is found, resolve it, and check further
    if (is_array($matchedRedirect)) {
        $GLOBALS['TYPO3_REQUEST'] = $request; // TODO <- ADD THIS LINE
        $url = $this->redirectService->getTargetUrl($matchedRedirect, $request->getQueryParams(), $request->getAttribute('frontend.user'), $request->getUri(), $request->getAttribute('site'));
        // [...]
    }
    // [...]

---
Besides this exception, all TS evaluation in case of a redirect will not correctly take request conditions into consideration. I cannot evaluate what's the impact of this

Related issues 1 (0 open — 1 closed)

Actions

Copy link

Updated by Stefan Bürk about 3 years ago

Assignee set to Stefan Bürk

Thanks for reporting this issue and a first analysis. Looking into the code this sounds reasonable. Will investigate this and work on a solution. Setting the global there should be avoided if possible.

Eventually do it for v10 in RedirectService->bootFrontendController(), where TypoScriptFrontendController is initialized, and reads the $GLOBAL['TYPO3_REQUEST'] in the __constructor() to keep that "moving of global" forward to the place the TSFE is earlier created than usually.

The point of evualting TypoScript condition is the other thing.

Note: Mentioned TypoScriptFrontendController->bootFrontednController() does not exists.

Note: This was fixed in master/v11 through using/passing original request / psr-7 request with following patch:
https://forge.typo3.org/issues/92666 | https://review.typo3.org/c/Packages/TYPO3.CMS/+/66140

Changing this would change signatures, so maybe the setting $GLOBALS['TYPO3_REQUEST'] could be the way to solve this for v10. (Still thinking about the right place).

Confirmed Can confirm this in v10.4.21 in running instance, master not confirmed (as fixed with stated patch above)

Note: This only leads to errin in FE Debugging context (Debug Context / FE Debugging enabled). In live context not (no exception thrown). Maybe that's the reason why I have some difficults to get a testcase (functional) for this running. (Failing to prove / cover the fix)

Actions

Copy link

Updated by Stefan Bürk about 3 years ago

Status changed from New to In Progress

Actions

Copy link

Updated by Gerrit Code Review about 3 years ago

Status changed from In Progress to Under Review

Patch set 1 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72074

Actions

Copy link

Updated by Gerrit Code Review about 3 years ago

Patch set 2 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72074

Actions

Copy link

Updated by Stefan Bürk about 3 years ago

Related to Task #92666: Use the current request to resolve TSFE in redirect resolving added

Actions

Copy link

Updated by Gerrit Code Review about 3 years ago

Patch set 3 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/72074

Actions

Copy link

Updated by Sybille Peters about 3 years ago

@Philip I can't reproduce this on latest 10.4 branch

Knowing how to reproduce this (with a minimal example) might be helpful for testing / reviewing the patch and debugging. Thank you.

test site with 3 languages, 2 fallback languages
set to debug preset
Added a redirect: /highlights => /features
on /features page added TS:

[siteLanguage("languageId") == 2 || 2 in siteLanguage("fallbackLanguageIds")]
[GLOBAL]

call page /highlights, redirects correctly without error.

Also tried with condition

[siteLanguage("languageId") == 2]
[GLOBAL]

Tested this both with page in default language and with translated page.

When the page with the condition is generated (regardless of the redirect), it does throw an exception (which is in log file):

Mon, 08 Nov 2021 22:34:06 +0100 [ERROR] request="5a6524d0d986c" component="TYPO3.CMS.Frontend.Configuration.TypoScript.ConditionMatching.ConditionMatcher": 
PHP Warning: in_array() expects parameter 2 to be array, null given in 
/var/www/t3coredev10x/vendor/symfony/expression-language/Node/BinaryNode.php line 100 - {"expression":"siteLanguage(\"languageId\") == 2 || 2 in siteLanguage(\"fallbackLanguageIds\")","exception":
"TYPO3\\CMS\\Core\\Error\\Exception: PHP Warning: in_array() expects parameter 2 to be array, null given in
/var/www/t3coredev10x/vendor/symfony/expression-language/Node/BinaryNode.php line 100 in 
/var/www/t3coredev10x/typo3/sysext/core/Classes/Error/ErrorHandler.php:134\nStack trace:

Not sure why, as mentioned I have 2 fallback languages.

Actions

Copy link

Updated by Stefan Bürk about 3 years ago

Sybille, I have reproduced that with 10.4-src (latest commit). I have also added a testcase for it which covers this (patchset 1). And the fix for it (patchset 3).

What you need to do, is to select a page as target ( LinkHandler uri t3://page?uid=1 ) Syntax. The issue in debug mode is produced there, as this needs the early bootstrapped TSFE, which is incomplete.
Anyway, there is a test which verifies this in the patch set.

Weired is your warning in "normal" as there the siteLanguage should be set (no redirect).

Actions

Copy link