From cd8a613c94a57c0016108a6fd39a35fef39d67ea Mon Sep 17 00:00:00 2001
From: Helmut Hummel <typo3@helhum.io>
Date: Fri, 1 May 2020 19:42:26 +0200
Subject: [PATCH] [BUGFIX] Consistently fetch SiteConfiguration from DI

SiteConfiguration is available from the DI container (both in failsafe and
symfony DI), therefore no constructor arguments MUST be passed to
GeneralUtility::makeInstance when fetching the SiteConfiguration singleton
instance, or a new instance will be created.

There are however multiple places where this object is still created
by passing constructor arguments to GeneralUtility::makeInstance
(this bypasses proxying to the container).
This leads to the situation that two different objects are created and
retrieved, depending on how it is fetched.

This is now fixed by changing each remaining retrieval via makeInstance
to NOT provide constructor arguments, which in turn results in a call to
the container to fetch the SiteConfiguration object.

Additionally the ServiceProvider introduced in #89892 is changed to allow
providing an alternative implementation using XCLASS. Although XCLASSES
are generally discouraged in favor of Services.yaml overwrites, there is
no other way to provide an alternative for this class, as it is registered
in an internal service provider.

Resolves: #91260
Releases: master
Change-Id: I8af1cafd737feccff9e06eacb23d6991105238d0
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64386
Tested-by: Benjamin Franzke <bfr@qbus.de>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Helmut Hummel <typo3@helhum.io>
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Helmut Hummel <typo3@helhum.io>
---
 .../Controller/SiteConfigurationController.php        |  5 ++---
 .../Form/FormDataProvider/SiteDatabaseEditRow.php     | 11 ++---------
 typo3/sysext/backend/Configuration/Services.yaml      |  2 ++
 .../core/Classes/Hooks/CreateSiteConfiguration.php    |  5 +----
 typo3/sysext/core/Classes/ServiceProvider.php         |  2 +-
 typo3/sysext/core/Classes/Site/SiteFinder.php         |  6 +-----
 .../Classes/Utility/InstallUtility.php                |  5 +----
 .../Tests/Unit/Utility/InstallUtilityTest.php         |  5 +++++
 .../Unit/ContentObject/ContentObjectRendererTest.php  |  4 ++++
 9 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/typo3/sysext/backend/Classes/Controller/SiteConfigurationController.php b/typo3/sysext/backend/Classes/Controller/SiteConfigurationController.php
index d8c0023421bd..c8785bd258d5 100644
--- a/typo3/sysext/backend/Classes/Controller/SiteConfigurationController.php
+++ b/typo3/sysext/backend/Classes/Controller/SiteConfigurationController.php
@@ -31,7 +31,6 @@ use TYPO3\CMS\Backend\Template\ModuleTemplate;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Configuration\SiteConfiguration;
-use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction;
 use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction;
@@ -329,7 +328,7 @@ class SiteConfigurationController
             $newSiteConfiguration = $this->validateFullStructure($newSysSiteData);
 
             // Persist the configuration
-            $siteConfigurationManager = GeneralUtility::makeInstance(SiteConfiguration::class, Environment::getConfigPath() . '/sites');
+            $siteConfigurationManager = GeneralUtility::makeInstance(SiteConfiguration::class);
             if (!$isNewConfiguration && $currentIdentifier !== $siteIdentifier) {
                 $siteConfigurationManager->rename($currentIdentifier, $siteIdentifier);
             }
@@ -560,7 +559,7 @@ class SiteConfigurationController
             throw new \RuntimeException('Not site identifier given', 1521565182);
         }
         // Verify site does exist, method throws if not
-        GeneralUtility::makeInstance(SiteConfiguration::class, Environment::getConfigPath() . '/sites')->delete($siteIdentifier);
+        GeneralUtility::makeInstance(SiteConfiguration::class)->delete($siteIdentifier);
         $uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
         $overviewRoute = $uriBuilder->buildUriFromRoute('site_configuration', ['action' => 'overview']);
         return new RedirectResponse($overviewRoute);
diff --git a/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php b/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php
index 5dbeeac92b21..562fc3379291 100644
--- a/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php
+++ b/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php
@@ -19,7 +19,6 @@ namespace TYPO3\CMS\Backend\Form\FormDataProvider;
 
 use TYPO3\CMS\Backend\Form\FormDataProviderInterface;
 use TYPO3\CMS\Core\Configuration\SiteConfiguration;
-use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Site\SiteFinder;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
@@ -35,15 +34,9 @@ class SiteDatabaseEditRow implements FormDataProviderInterface
      */
     protected $siteConfiguration;
 
-    /**
-     * @param SiteConfiguration $siteConfiguration
-     */
-    public function __construct(SiteConfiguration $siteConfiguration = null)
+    public function __construct(SiteConfiguration $siteConfiguration)
     {
-        $this->siteConfiguration = $siteConfiguration ?? GeneralUtility::makeInstance(
-            SiteConfiguration::class,
-            Environment::getConfigPath() . '/sites'
-        );
+        $this->siteConfiguration = $siteConfiguration;
     }
 
     /**
diff --git a/typo3/sysext/backend/Configuration/Services.yaml b/typo3/sysext/backend/Configuration/Services.yaml
index fde9a305964a..48dfef702aac 100644
--- a/typo3/sysext/backend/Configuration/Services.yaml
+++ b/typo3/sysext/backend/Configuration/Services.yaml
@@ -48,6 +48,8 @@ services:
   TYPO3\CMS\Backend\Controller\HelpController:
     tags: ['backend.controller']
 
+  TYPO3\CMS\Backend\Form\FormDataProvider\SiteDatabaseEditRow:
+    public: true
 
   # Listener for old Signal Slots
   TYPO3\CMS\Backend\Compatibility\SlotReplacement:
diff --git a/typo3/sysext/core/Classes/Hooks/CreateSiteConfiguration.php b/typo3/sysext/core/Classes/Hooks/CreateSiteConfiguration.php
index d3c9b1e155a0..0b2d5901bb0f 100644
--- a/typo3/sysext/core/Classes/Hooks/CreateSiteConfiguration.php
+++ b/typo3/sysext/core/Classes/Hooks/CreateSiteConfiguration.php
@@ -77,10 +77,7 @@ class CreateSiteConfiguration
         $siteIdentifier = $entryPoint . '-' . GeneralUtility::shortMD5((string)$pageId);
 
         if (!$this->siteExistsByRootPageId($pageId)) {
-            $siteConfiguration = GeneralUtility::makeInstance(
-                SiteConfiguration::class,
-                Environment::getConfigPath() . '/sites'
-            );
+            $siteConfiguration = GeneralUtility::makeInstance(SiteConfiguration::class);
             $normalizedParams = $this->getNormalizedParams();
             $basePrefix = Environment::isCli() ? $normalizedParams->getSitePath() : $normalizedParams->getSiteUrl();
             $siteConfiguration->createNewBasicSite(
diff --git a/typo3/sysext/core/Classes/ServiceProvider.php b/typo3/sysext/core/Classes/ServiceProvider.php
index 3cdb5e0e0f49..70bf05170760 100644
--- a/typo3/sysext/core/Classes/ServiceProvider.php
+++ b/typo3/sysext/core/Classes/ServiceProvider.php
@@ -112,7 +112,7 @@ class ServiceProvider extends AbstractServiceProvider
 
     public static function getSiteConfiguration(ContainerInterface $container): Configuration\SiteConfiguration
     {
-        return new Configuration\SiteConfiguration(Environment::getConfigPath() . '/sites');
+        return self::new($container, Configuration\SiteConfiguration::class, [Environment::getConfigPath() . '/sites']);
     }
 
     public static function getConsoleCommandApplication(ContainerInterface $container): Console\CommandApplication
diff --git a/typo3/sysext/core/Classes/Site/SiteFinder.php b/typo3/sysext/core/Classes/Site/SiteFinder.php
index 1feadcf25d0b..5cbf14a66182 100644
--- a/typo3/sysext/core/Classes/Site/SiteFinder.php
+++ b/typo3/sysext/core/Classes/Site/SiteFinder.php
@@ -18,7 +18,6 @@ declare(strict_types=1);
 namespace TYPO3\CMS\Core\Site;
 
 use TYPO3\CMS\Core\Configuration\SiteConfiguration;
-use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Exception\Page\PageNotFoundException;
 use TYPO3\CMS\Core\Exception\SiteNotFoundException;
 use TYPO3\CMS\Core\Site\Entity\Site;
@@ -54,10 +53,7 @@ class SiteFinder
      */
     public function __construct(SiteConfiguration $siteConfiguration = null)
     {
-        $this->siteConfiguration = $siteConfiguration ?? GeneralUtility::makeInstance(
-            SiteConfiguration::class,
-            Environment::getConfigPath() . '/sites'
-        );
+        $this->siteConfiguration = $siteConfiguration ?? GeneralUtility::makeInstance(SiteConfiguration::class);
         $this->fetchAllSites();
     }
 
diff --git a/typo3/sysext/extensionmanager/Classes/Utility/InstallUtility.php b/typo3/sysext/extensionmanager/Classes/Utility/InstallUtility.php
index af072b9e1dc9..e2cc1bcc38e6 100644
--- a/typo3/sysext/extensionmanager/Classes/Utility/InstallUtility.php
+++ b/typo3/sysext/extensionmanager/Classes/Utility/InstallUtility.php
@@ -590,10 +590,7 @@ class InstallUtility implements SingletonInterface, LoggerAwareInterface
             return;
         }
 
-        $siteConfiguration = GeneralUtility::makeInstance(
-            SiteConfiguration::class,
-            $destinationFolder
-        );
+        $siteConfiguration = GeneralUtility::makeInstance(SiteConfiguration::class);
         $existingSites = $siteConfiguration->resolveAllExistingSites(false);
 
         GeneralUtility::mkdir($destinationFolder);
diff --git a/typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php b/typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php
index ce3ace2bec1c..5b9f55eac5c5 100644
--- a/typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php
+++ b/typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php
@@ -23,6 +23,7 @@ use Psr\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Yaml\Yaml;
 use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Cache\Frontend\NullFrontend;
+use TYPO3\CMS\Core\Configuration\SiteConfiguration;
 use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Registry;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -310,6 +311,8 @@ class InstallUtilityTest extends UnitTestCase
         GeneralUtility::mkdir_deep($absPath . 'Initialisation/Site/' . $siteIdentifier);
         file_put_contents($absPath . 'Initialisation/Site/' . $siteIdentifier . '/config.yaml', $config);
 
+        GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
+
         $subject = new InstallUtility();
         $subject->injectEventDispatcher($this->prophesize(EventDispatcherInterface::class)->reveal());
         $listUtility = $this->prophesize(ListUtility::class);
@@ -376,6 +379,8 @@ class InstallUtilityTest extends UnitTestCase
         GeneralUtility::mkdir_deep($configDir . '/sites/' . $siteIdentifier);
         file_put_contents($configDir . '/' . $existingSiteConfig, $config);
 
+        GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
+
         $subject = new InstallUtility();
         $subject->injectEventDispatcher($this->prophesize(EventDispatcherInterface::class)->reveal());
         $listUtility = $this->prophesize(ListUtility::class);
diff --git a/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php b/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
index 55f8240ad751..04d0886ebfcb 100644
--- a/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
@@ -23,6 +23,7 @@ use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface as CacheFrontendInterface;
 use TYPO3\CMS\Core\Cache\Frontend\NullFrontend;
+use TYPO3\CMS\Core\Configuration\SiteConfiguration;
 use TYPO3\CMS\Core\Context\Context;
 use TYPO3\CMS\Core\Context\UserAspect;
 use TYPO3\CMS\Core\Context\WorkspaceAspect;
@@ -2712,6 +2713,8 @@ class ContentObjectRendererTest extends UnitTestCase
         $this->cacheManager->getCache('runtime')->willReturn(new NullFrontend('dummy'));
         $this->cacheManager->getCache('core')->willReturn(new NullFrontend('dummy'));
 
+        GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
+
         $this->subject->_set('typoScriptFrontendController', $typoScriptFrontendControllerMockObject);
 
         self::assertEquals($expectedResult, $this->subject->typoLink($linkText, $configuration));
@@ -3158,6 +3161,7 @@ class ContentObjectRendererTest extends UnitTestCase
     {
         $this->cacheManager->getCache('runtime')->willReturn(new NullFrontend('runtime'));
         $this->cacheManager->getCache('core')->willReturn(new NullFrontend('runtime'));
+        GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
         $linkText = 'Nice Text';
         $configuration = [
             'parameter' => 'https://example.com 13x84:target=myexample'
-- 
GitLab