From 0dd2f76d6ad8d30bf9d6f28f660fb9b2a9f8613d Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Fri, 11 Nov 2022 15:37:04 +0100
Subject: [PATCH] [TASK] Have site attribute in filelist module
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the Site construct has been added to TYPO3
in v9, the setup together with PseudoSites was
pretty complex. This has been simplified meanwhile
and the resolved site is usually added to the
backend request as attribute.

Except in cases where GET / POST param "id" is
misused. This is the case in filelist module.

The patch changes the backend related SiteResolver
middleware to *always* add a site attribute to
the request, backend controllers can now rely
on it to be set, even if its just a NullSite.

Resolves: #99121
Related: #86153
Releases: main
Change-Id: I63ea9ed1b5bca5db298f7a3bf000fcb4f777f627
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/76665
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
---
 .../Classes/Middleware/SiteResolver.php       | 42 +++++++++----------
 .../Middleware/SiteResolverTest.php           | 31 ++++++--------
 .../core/Classes/Routing/SiteMatcher.php      |  3 +-
 3 files changed, 33 insertions(+), 43 deletions(-)
 rename typo3/sysext/backend/Tests/{Unit => Functional}/Middleware/SiteResolverTest.php (57%)

diff --git a/typo3/sysext/backend/Classes/Middleware/SiteResolver.php b/typo3/sysext/backend/Classes/Middleware/SiteResolver.php
index 73a2993f87b8..1de5e1415168 100644
--- a/typo3/sysext/backend/Classes/Middleware/SiteResolver.php
+++ b/typo3/sysext/backend/Classes/Middleware/SiteResolver.php
@@ -35,37 +35,33 @@ use TYPO3\CMS\Core\Utility\MathUtility;
  */
 class SiteResolver implements MiddlewareInterface
 {
-    /**
-     * @var SiteMatcher
-     */
-    protected $siteMatcher;
-
-    public function __construct(SiteMatcher $siteMatcher)
-    {
-        $this->siteMatcher = $siteMatcher;
+    public function __construct(
+        private readonly SiteMatcher $siteMatcher
+    ) {
     }
 
     /**
-     * Resolve the site information by checking the page ID ("id" parameter) which is typically used in BE modules
-     * of type "web".
-     *
-     * @param ServerRequestInterface $request
-     * @param RequestHandlerInterface $handler
-     * @return ResponseInterface
+     * Resolve the site information by checking the page ID ("id" parameter) which is typically
+     * used in BE modules of type "web".
      */
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $pageId = ($request->getQueryParams()['id'] ?? $request->getParsedBody()['id'] ?? 0);
-        // Check if we have a numeric _GET/_POST parameter for "id", then a site information can be resolved based.
-        if (MathUtility::canBeInterpretedAsInteger($pageId)) {
-            $pageId = (int)$pageId;
-            $rootLine = null;
-            if ($pageId > 0) {
-                $rootLine = BackendUtility::BEgetRootLine($pageId);
-            }
-            $site = $this->siteMatcher->matchByPageId($pageId, $rootLine);
-            $request = $request->withAttribute('site', $site);
+        if (!MathUtility::canBeInterpretedAsInteger($pageId)) {
+            // @todo: The "filelist" module abuses "id" to carry a storage like "1:/" around. This
+            //        should be changed. To *always* have a site attribute attached to the request,
+            //        we for now resolve to zero here, leading to NullSite object.
+            //        Change "filelist" module to no longer abuse "id" GET argument and throw an
+            //        exception here if $pageUid can not be resolved to an int.
+            $pageId = 0;
+        }
+        $pageId = (int)$pageId;
+        $rootLine = null;
+        if ($pageId > 0) {
+            $rootLine = BackendUtility::BEgetRootLine($pageId);
         }
+        $site = $this->siteMatcher->matchByPageId($pageId, $rootLine);
+        $request = $request->withAttribute('site', $site);
         return $handler->handle($request);
     }
 }
diff --git a/typo3/sysext/backend/Tests/Unit/Middleware/SiteResolverTest.php b/typo3/sysext/backend/Tests/Functional/Middleware/SiteResolverTest.php
similarity index 57%
rename from typo3/sysext/backend/Tests/Unit/Middleware/SiteResolverTest.php
rename to typo3/sysext/backend/Tests/Functional/Middleware/SiteResolverTest.php
index 51825ea3e8d2..e5f1f45d5f2a 100644
--- a/typo3/sysext/backend/Tests/Unit/Middleware/SiteResolverTest.php
+++ b/typo3/sysext/backend/Tests/Functional/Middleware/SiteResolverTest.php
@@ -15,7 +15,7 @@ declare(strict_types=1);
  * The TYPO3 project - inspiring people to share!
  */
 
-namespace TYPO3\CMS\Backend\Tests\Unit\Middleware;
+namespace TYPO3\CMS\Backend\Tests\Functional\Middleware;
 
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
@@ -23,36 +23,31 @@ use Psr\Http\Server\RequestHandlerInterface;
 use TYPO3\CMS\Backend\Middleware\SiteResolver;
 use TYPO3\CMS\Core\Http\JsonResponse;
 use TYPO3\CMS\Core\Http\ServerRequest;
-use TYPO3\CMS\Core\Routing\SiteMatcher;
-use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+use TYPO3\CMS\Core\Site\Entity\NullSite;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
 
-class SiteResolverTest extends UnitTestCase
+class SiteResolverTest extends FunctionalTestCase
 {
     /**
      * @test
      */
-    public function requestIsNotModifiedIfPageIdParameterIsNoInteger(): void
+    public function requestHasNullSiteAttributeIfIdParameterIsNoInteger(): void
     {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1668696350);
         $incomingUrl = 'http://localhost:8080/typo3/module/file/FilelistList?token=d7d864db2b26c1d0f0537718b16890f336f4af2b&id=9831:/styleguide/';
-
-        $siteMatcherMock = $this->createMock(SiteMatcher::class);
-        $subject = new SiteResolver($siteMatcherMock);
-
+        $subject = $this->get(SiteResolver::class);
         $incomingRequest = new ServerRequest($incomingUrl, 'GET');
         $incomingRequest = $incomingRequest->withQueryParams(['id' => '9831:/styleguide/']);
         $requestHandler = new class () implements RequestHandlerInterface {
-            public ServerRequestInterface $incomingRequest;
             public function handle(ServerRequestInterface $request): ResponseInterface
             {
-                return new JsonResponse([], $request === $this->incomingRequest ? 200 : 500);
-            }
-            public function setIncomingRequest(ServerRequestInterface $incomingRequest): void
-            {
-                $this->incomingRequest = $incomingRequest;
+                if ($request->getAttribute('site') instanceof NullSite) {
+                    throw new \RuntimeException('testing', 1668696350);
+                }
+                return new JsonResponse();
             }
         };
-        $requestHandler->setIncomingRequest($incomingRequest);
-        $response = $subject->process($incomingRequest, $requestHandler);
-        self::assertEquals(200, $response->getStatusCode());
+        $subject->process($incomingRequest, $requestHandler);
     }
 }
diff --git a/typo3/sysext/core/Classes/Routing/SiteMatcher.php b/typo3/sysext/core/Classes/Routing/SiteMatcher.php
index ec1b92e9fd51..48fea05844b5 100644
--- a/typo3/sysext/core/Classes/Routing/SiteMatcher.php
+++ b/typo3/sysext/core/Classes/Routing/SiteMatcher.php
@@ -176,13 +176,12 @@ class SiteMatcher implements SingletonInterface
      * @param int $pageId uid of a page in default language
      * @param array|null $rootLine an alternative root line, if already at and.
      * @return SiteInterface
-     * @throws SiteNotFoundException
      */
     public function matchByPageId(int $pageId, array $rootLine = null): SiteInterface
     {
         try {
             return $this->finder->getSiteByPageId($pageId, $rootLine);
-        } catch (SiteNotFoundException $e) {
+        } catch (SiteNotFoundException) {
             return new NullSite();
         }
     }
-- 
GitLab