Bug #66278
closed
RequestHandler creates $GLOBALS['TSFE'] incomplete
100%
Description
I am not sure how to reproduce the original issue, but here is my story:
I was testing https://review.typo3.org/#/c/8719/, so I followed the reproduce instruction in the ticket.
Some time everything went well, but all of a sudden my frontend responded with two errors:
Warning: Invalid argument supplied for foreach() in /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php on line 4095
Call Stack: 0.0000 238920
1. {main}() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/index.php:0 0.0023 1222392
2. TYPO3\CMS\Core\Core\Bootstrap->run() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/index.php:33 0.0202 2775632
3. TYPO3\CMS\Frontend\RequestHandler->handleRequest() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/core/Classes/Core/Bootstrap.php:195 0.4188 6539192
4. TYPO3\CMS\Backend\FrontendBackendUserAuthentication->initializeFrontendEdit() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/frontend/Classes/RequestHandler.php:150 0.4188 6539592
5. TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->getPagesTSconfig() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php:123
Fatal error: Call to a member function getHash() on a non-object in /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php on line 4102
Call Stack: 0.0000 238920
1. {main}() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/index.php:0 0.0023 1222392
2. TYPO3\CMS\Core\Core\Bootstrap->run() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/index.php:33 0.0202 2775632
3. TYPO3\CMS\Frontend\RequestHandler->handleRequest() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/core/Classes/Core/Bootstrap.php:195 0.4188 6539192
4. TYPO3\CMS\Backend\FrontendBackendUserAuthentication->initializeFrontendEdit() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/frontend/Classes/RequestHandler.php:150 0.4188 6539592
5. TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->getPagesTSconfig() /var/www/typo3-testing.de/vhosts/master/htdocs/TYPO3.CMS/typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php:123
After some investigation we found out, that the request handler rebrush (#65914) was the change that introduced this error.
Even after undoing all the changes I did in my environment, hard resetting the master branch, flushing all the caches several times, for me the error persists.
Changing the line 360 in typo3/sysext/frontend/Classes/RequestHandler.php from$GLOBALS['TSFE'] = $this->controller; to $GLOBALS['TSFE'] = &$this->controller; brought my frontend back.
Updated by Gerrit Code Review over 9 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/38489
Updated by Jigal van Hemert over 9 years ago
A bit of background of the issue: objects in PHP5 are stored outside the variables and the variable contains an identifier of that object. Copying a variable that "contains" an object only copies the identifier of the actual object. A reference to a variable that "contains" an object creates an alias to that variable (two names for the same variable in memory).
The objects involved in the RequestHandler are quite complex. TypoScriptFrontendController contains sys_page (PageRepository) and some of the functions in that class use methods of $GLOBALS['TSFE'] (TypoScriptFrontendController).
Although the copy action that is changed in this patch should copy an identifier to the object there appears to be a difference between creating a copy and creating a reference (tested in PHP 5.5.9 and 5.5.15).
Perhaps it would be wise to use references in general because references work at a slightly lower level than copies.
Updated by Anja Leichsenring over 9 years ago
Another little detail: the failure occures only while a cookie named AMDCMD_prev is present in the frontend. Removing this cookie brings the Frontend back to function.
Updated by Markus Klein over 9 years ago
I'm failing to reproduce this on PHP 5.6.7
Updated by Markus Klein over 9 years ago
- Category changed from Frontend to Workspaces
The problem is the preview hook in versioning.
\TYPO3\CMS\Version\Hook\PreviewHook::checkForPreview
There $GLOBALS['TSFE'] is overridden.
The hook is called in
typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php:1212
via
typo3/sysext/frontend/Classes/RequestHandler.php:112
At this point in execution (RequestHandler.php:112) $this->controller and $GLOBALS['TSFE'] do not match anymore.
The current patch "solves" this by enabling modification of $this->controller through the reference $GLOBALS['TSFE'].
This is indeed a quick and dirty solution for the problem, albeit it completely violates all rules of good software design.
Updated by Jigal van Hemert over 9 years ago
The real issue here is that we use a global variable and that this is seen as a design flaw nowadays. In combination with the rather unintuitive handling of objects and references in PHP the side effects are often hard to grasp.
In short:
- variables that hold an object actually contain the identifier of that object
- when such a variable is copied only the identifier is copied to the new variable
- references to variables are actually aliases; two names for the same variable in memory
$this->controller and $GLOBALS['TSFE'] are two variables contain the identifier of the same object. In the hook $GLOBALS['TSFE'] is replaced and both variables now contain the identifiers of two different objects.
If we use a reference instead of a copy then $this->controller and $GLOBALS['TSFE'] are merely two names for the same variable containing the identifier of the object. When the global variable is changed $this->controller will point to this new object.
We should be very careful when copying/replacing objects.
Updated by Gerrit Code Review over 9 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/38489
Updated by Markus Klein over 9 years ago
- Sprint Focus set to Stabilization Sprint
Updated by Anja Leichsenring over 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset e39783929bd8483f3a6b074c7931e7e8b1ffd176.
Updated by Riccardo De Contardi about 7 years ago
- Status changed from Resolved to Closed