From 5601673dd96b886b76a25d977db00435e1bad26f Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <bfr@qbus.de>
Date: Fri, 22 Nov 2019 07:26:43 +0100
Subject: [PATCH] [BUGFIX] Provide global Symfony container instance in upgrade
wizards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Upgrade wizars may contain legacy code that requires services
using makeInstance. As makeInstance needs to use the PSR-11 container
for services that require dependency injection, we need to provide
the global container intance during upgrade wizard execution.
Also ensure that only *one* (singleton) PackageManager instance is used:
ProviderConfigurationLoader used to create a second PackageManager
instance. It passed a constructor argument which caused GU::makeInstance
to bypass the symfony (PSR-11) container and to create a new instance
(on its own). This new instance was then cached as singleton instance and
all subsequent makeInstance calls returned this instance instead of the
global instance. Therefore we now ensure that new singleton instances
will not be cached if the singleton is known by the PSR-11 container.
This inconsistency caused the Typo3DbExtractionUpdate to fail because
some codepaths used one PackageManager instance (through DI) and some
codepaths another instance (through makeInstance).
Change-Id: I947310d1c2e46f7ded6399ad48d94efa9854f347
Releases: master
Resolves: #89504
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62425
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Susanne Moog <look@susi.dev>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Jörg Bösche <typo3@joergboesche.de>
Reviewed-by: Susanne Moog <look@susi.dev>
---
typo3/sysext/core/Classes/Core/Bootstrap.php | 5 +++++
.../ProviderConfigurationLoader.php | 6 +-----
.../core/Classes/Utility/GeneralUtility.php | 4 ++--
.../ContentObjectRendererTest.php | 3 +++
.../Classes/Controller/AbstractController.php | 10 +++++++--
.../Classes/Controller/UpgradeController.php | 21 ++++++++++++-------
.../Classes/Service/LateBootService.php | 7 +++++--
7 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/typo3/sysext/core/Classes/Core/Bootstrap.php b/typo3/sysext/core/Classes/Core/Bootstrap.php
index cf18dd731464..f8969689daec 100644
--- a/typo3/sysext/core/Classes/Core/Bootstrap.php
+++ b/typo3/sysext/core/Classes/Core/Bootstrap.php
@@ -132,6 +132,11 @@ class Bootstrap
// makeInstance() method creates classes using the container from now on.
GeneralUtility::setContainer($container);
+ // Reset singleton instances in order for GeneralUtility::makeInstance() to use
+ // ContainerInterface->get() for early services from now on.
+ GeneralUtility::removeSingletonInstance(LogManager::class, $logManager);
+ GeneralUtility::removeSingletonInstance(PackageManager::class, $packageManager);
+
if ($failsafe) {
$bootState->done = true;
return $container;
diff --git a/typo3/sysext/core/Classes/ExpressionLanguage/ProviderConfigurationLoader.php b/typo3/sysext/core/Classes/ExpressionLanguage/ProviderConfigurationLoader.php
index e90ab08a50c8..51f9e2071df3 100644
--- a/typo3/sysext/core/Classes/ExpressionLanguage/ProviderConfigurationLoader.php
+++ b/typo3/sysext/core/Classes/ExpressionLanguage/ProviderConfigurationLoader.php
@@ -5,7 +5,6 @@ namespace TYPO3\CMS\Core\ExpressionLanguage;
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Package\PackageManager;
-use TYPO3\CMS\Core\Service\DependencyOrderingService;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
@@ -22,10 +21,7 @@ class ProviderConfigurationLoader
*/
public function getExpressionLanguageProviders(): array
{
- $packageManager = GeneralUtility::makeInstance(
- PackageManager::class,
- GeneralUtility::makeInstance(DependencyOrderingService::class)
- );
+ $packageManager = GeneralUtility::makeInstance(PackageManager::class);
$cache = GeneralUtility::makeInstance(CacheManager::class)->getCache('core');
if ($cache->has($this->cacheIdentifier)) {
diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
index 7d4c71be1efb..dd4fd6491211 100644
--- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php
+++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
@@ -3459,8 +3459,8 @@ class GeneralUtility
// Create new instance and call constructor with parameters
$instance = new $finalClassName(...$constructorArguments);
- // Register new singleton instance
- if ($instance instanceof SingletonInterface) {
+ // Register new singleton instance, but only if it is not a known PSR-11 container service
+ if ($instance instanceof SingletonInterface && !(self::$container !== null && self::$container->has($className))) {
self::$singletonInstances[$finalClassName] = $instance;
}
if ($instance instanceof LoggerAwareInterface) {
diff --git a/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php b/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
index defd00bcb064..627e6c44499a 100644
--- a/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
@@ -37,6 +37,7 @@ use TYPO3\CMS\Core\Resource\Exception\InvalidPathException;
use TYPO3\CMS\Core\Resource\File;
use TYPO3\CMS\Core\Resource\ResourceFactory;
use TYPO3\CMS\Core\Resource\ResourceStorage;
+use TYPO3\CMS\Core\Service\DependencyOrderingService;
use TYPO3\CMS\Core\Site\Entity\Site;
use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
use TYPO3\CMS\Core\TimeTracker\TimeTracker;
@@ -1710,6 +1711,8 @@ class ContentObjectRendererTest extends UnitTestCase
*/
public function getDataWithTypeSiteWithBaseVariants(): void
{
+ $packageManager = new PackageManager(new DependencyOrderingService);
+ GeneralUtility::setSingletonInstance(PackageManager::class, $packageManager);
$cacheManagerProphecy = $this->prophesize(CacheManager::class);
$cacheManagerProphecy->getCache('core')->willReturn(new NullFrontend('core'));
GeneralUtility::setSingletonInstance(CacheManager::class, $cacheManagerProphecy->reveal());
diff --git a/typo3/sysext/install/Classes/Controller/AbstractController.php b/typo3/sysext/install/Classes/Controller/AbstractController.php
index 85c4fca6aa16..b72a63726179 100644
--- a/typo3/sysext/install/Classes/Controller/AbstractController.php
+++ b/typo3/sysext/install/Classes/Controller/AbstractController.php
@@ -59,10 +59,16 @@ class AbstractController
* Those actions can potentially fatal if some old extension is loaded that triggers
* a fatal in ext_localconf or ext_tables code! Use only if really needed.
*
+ * @param bool $resetContainer
* @return ContainerInterface
*/
- protected function loadExtLocalconfDatabaseAndExtTables(): ContainerInterface
+ public function loadExtLocalconfDatabaseAndExtTables(bool $resetContainer = true): ContainerInterface
{
- return GeneralUtility::makeInstance(LateBootService::class)->loadExtLocalconfDatabaseAndExtTables();
+ return GeneralUtility::makeInstance(LateBootService::class)->loadExtLocalconfDatabaseAndExtTables($resetContainer);
+ }
+
+ public function resetGlobalContainer(): void
+ {
+ GeneralUtility::makeInstance(LateBootService::class)->makeCurrent(null, []);
}
}
diff --git a/typo3/sysext/install/Classes/Controller/UpgradeController.php b/typo3/sysext/install/Classes/Controller/UpgradeController.php
index 8c263954e883..1cce33f25780 100644
--- a/typo3/sysext/install/Classes/Controller/UpgradeController.php
+++ b/typo3/sysext/install/Classes/Controller/UpgradeController.php
@@ -916,9 +916,10 @@ class UpgradeController extends AbstractController
public function upgradeWizardsBlockingDatabaseAddsAction(): ResponseInterface
{
// ext_localconf, db and ext_tables must be loaded for the updates :(
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$upgradeWizardsService = new UpgradeWizardsService();
$adds = $upgradeWizardsService->getBlockingDatabaseAdds();
+ $this->resetGlobalContainer();
$needsUpdate = false;
if (!empty($adds)) {
$needsUpdate = true;
@@ -938,9 +939,10 @@ class UpgradeController extends AbstractController
public function upgradeWizardsBlockingDatabaseExecuteAction(): ResponseInterface
{
// ext_localconf, db and ext_tables must be loaded for the updates :(
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$upgradeWizardsService = new UpgradeWizardsService();
$upgradeWizardsService->addMissingTablesAndFields();
+ $this->resetGlobalContainer();
$messages = new FlashMessageQueue('install');
$messages->enqueue(new FlashMessage(
'',
@@ -994,10 +996,11 @@ class UpgradeController extends AbstractController
*/
public function upgradeWizardsDoneUpgradesAction(): ResponseInterface
{
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$upgradeWizardsService = new UpgradeWizardsService();
$wizardsDone = $upgradeWizardsService->listOfWizardsDone();
$rowUpdatersDone = $upgradeWizardsService->listOfRowUpdatersDone();
+ $this->resetGlobalContainer();
$messages = new FlashMessageQueue('install');
if (empty($wizardsDone) && empty($rowUpdatersDone)) {
$messages->enqueue(new FlashMessage(
@@ -1022,10 +1025,11 @@ class UpgradeController extends AbstractController
public function upgradeWizardsExecuteAction(ServerRequestInterface $request): ResponseInterface
{
// ext_localconf, db and ext_tables must be loaded for the updates :(
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$upgradeWizardsService = new UpgradeWizardsService();
$identifier = $request->getParsedBody()['install']['identifier'];
$messages = $upgradeWizardsService->executeWizard($identifier);
+ $this->resetGlobalContainer();
return new JsonResponse([
'success' => true,
'status' => $messages,
@@ -1041,10 +1045,11 @@ class UpgradeController extends AbstractController
public function upgradeWizardsInputAction(ServerRequestInterface $request): ResponseInterface
{
// ext_localconf, db and ext_tables must be loaded for the updates :(
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$upgradeWizardsService = new UpgradeWizardsService();
$identifier = $request->getParsedBody()['install']['identifier'];
$result = $upgradeWizardsService->getWizardUserInput($identifier);
+ $this->resetGlobalContainer();
return new JsonResponse([
'success' => true,
'status' => [],
@@ -1060,9 +1065,10 @@ class UpgradeController extends AbstractController
public function upgradeWizardsListAction(): ResponseInterface
{
// ext_localconf, db and ext_tables must be loaded for the updates :(
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$upgradeWizardsService = new UpgradeWizardsService();
$wizards = $upgradeWizardsService->getUpgradeWizardsList();
+ $this->resetGlobalContainer();
return new JsonResponse([
'success' => true,
'status' => [],
@@ -1078,10 +1084,11 @@ class UpgradeController extends AbstractController
*/
public function upgradeWizardsMarkUndoneAction(ServerRequestInterface $request): ResponseInterface
{
- $this->loadExtLocalconfDatabaseAndExtTables();
+ $this->loadExtLocalconfDatabaseAndExtTables(false);
$wizardToBeMarkedAsUndoneIdentifier = $request->getParsedBody()['install']['identifier'];
$upgradeWizardsService = new UpgradeWizardsService();
$result = $upgradeWizardsService->markWizardUndone($wizardToBeMarkedAsUndoneIdentifier);
+ $this->resetGlobalContainer();
$messages = new FlashMessageQueue('install');
if ($result) {
$messages->enqueue(new FlashMessage(
diff --git a/typo3/sysext/install/Classes/Service/LateBootService.php b/typo3/sysext/install/Classes/Service/LateBootService.php
index fe7223d0c45b..291d486eafac 100644
--- a/typo3/sysext/install/Classes/Service/LateBootService.php
+++ b/typo3/sysext/install/Classes/Service/LateBootService.php
@@ -114,9 +114,10 @@ class LateBootService
/**
* Bootstrap a non-failsafe container and load ext_localconf
*
+ * @param bool $resetContainer
* @return ContainerInterface
*/
- public function loadExtLocalconfDatabaseAndExtTables(): ContainerInterface
+ public function loadExtLocalconfDatabaseAndExtTables(bool $resetContainer = true): ContainerInterface
{
$container = $this->getContainer();
@@ -134,7 +135,9 @@ class LateBootService
Bootstrap::loadBaseTca(false);
Bootstrap::loadExtTables(false);
- $this->makeCurrent(null, $backup);
+ if ($resetContainer) {
+ $this->makeCurrent(null, $backup);
+ }
return $container;
}
--
GitLab