From 80b29e6cdae09a4352b897fc7d7681f2f256bcee Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Sun, 27 May 2018 17:11:46 +0200
Subject: [PATCH] [BUGFIX] Clean up error handler for site handling

This patch handles several issues related to the new
site error handling introduced in TYPO3 v9.2.0:

1. It adds unit tests
2. It adds a missing exception when an error handler
is configured which does not implement the PageErrorHandlerInterface
3. It fixes one minor issue in PageContentErrorHandler
where a wrong "InvalidArgumentException" was thrown
4. All PageErrorHandler logic was moved from EXT:frontend
to EXT:core, as this would be a penalty across packages -
the "Site" entity depends on them, so they must go to the same
package (core).

Releases: master
Resolves: #85101
Change-Id: Ibdc05024abd7c719dd8d5dcb3388bf1679b69990
Reviewed-on: https://review.typo3.org/57057
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
---
 .../FluidPageErrorHandler.php                 |   2 +-
 .../InvalidPageErrorHandlerException.php      |  27 ++++
 .../PageContentErrorHandler.php               |   7 +-
 .../PageErrorHandlerInterface.php             |   8 +-
 .../sysext/core/Classes/Site/Entity/Site.php  |  26 ++--
 .../core/Tests/Unit/Site/Entity/SiteTest.php  | 116 ++++++++++++++++++
 6 files changed, 166 insertions(+), 20 deletions(-)
 rename typo3/sysext/{frontend/Classes => core/Classes/Error}/PageErrorHandler/FluidPageErrorHandler.php (98%)
 create mode 100644 typo3/sysext/core/Classes/Error/PageErrorHandler/InvalidPageErrorHandlerException.php
 rename typo3/sysext/{frontend/Classes => core/Classes/Error}/PageErrorHandler/PageContentErrorHandler.php (92%)
 rename typo3/sysext/{frontend/Classes => core/Classes/Error}/PageErrorHandler/PageErrorHandlerInterface.php (77%)
 create mode 100644 typo3/sysext/core/Tests/Unit/Site/Entity/SiteTest.php

diff --git a/typo3/sysext/frontend/Classes/PageErrorHandler/FluidPageErrorHandler.php b/typo3/sysext/core/Classes/Error/PageErrorHandler/FluidPageErrorHandler.php
similarity index 98%
rename from typo3/sysext/frontend/Classes/PageErrorHandler/FluidPageErrorHandler.php
rename to typo3/sysext/core/Classes/Error/PageErrorHandler/FluidPageErrorHandler.php
index 074af5a8f5d9..80aa76d32f4a 100644
--- a/typo3/sysext/frontend/Classes/PageErrorHandler/FluidPageErrorHandler.php
+++ b/typo3/sysext/core/Classes/Error/PageErrorHandler/FluidPageErrorHandler.php
@@ -1,7 +1,7 @@
 <?php
 declare(strict_types = 1);
 
-namespace TYPO3\CMS\Frontend\PageErrorHandler;
+namespace TYPO3\CMS\Core\Error\PageErrorHandler;
 
 /*
  * This file is part of the TYPO3 CMS project.
diff --git a/typo3/sysext/core/Classes/Error/PageErrorHandler/InvalidPageErrorHandlerException.php b/typo3/sysext/core/Classes/Error/PageErrorHandler/InvalidPageErrorHandlerException.php
new file mode 100644
index 000000000000..2e2cb66d6071
--- /dev/null
+++ b/typo3/sysext/core/Classes/Error/PageErrorHandler/InvalidPageErrorHandlerException.php
@@ -0,0 +1,27 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Core\Error\PageErrorHandler;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Core\Error\Exception;
+
+/**
+ * Is typically used, when a site configuration has a page-error handler configured but this does not implement
+ * the PageErrorHandlerInterface
+ */
+class InvalidPageErrorHandlerException extends Exception
+{
+}
diff --git a/typo3/sysext/frontend/Classes/PageErrorHandler/PageContentErrorHandler.php b/typo3/sysext/core/Classes/Error/PageErrorHandler/PageContentErrorHandler.php
similarity index 92%
rename from typo3/sysext/frontend/Classes/PageErrorHandler/PageContentErrorHandler.php
rename to typo3/sysext/core/Classes/Error/PageErrorHandler/PageContentErrorHandler.php
index 3e5a32c2174a..c68900382f1c 100644
--- a/typo3/sysext/frontend/Classes/PageErrorHandler/PageContentErrorHandler.php
+++ b/typo3/sysext/core/Classes/Error/PageErrorHandler/PageContentErrorHandler.php
@@ -1,7 +1,7 @@
 <?php
 declare(strict_types = 1);
 
-namespace TYPO3\CMS\Frontend\PageErrorHandler;
+namespace TYPO3\CMS\Core\Error\PageErrorHandler;
 
 /*
  * This file is part of the TYPO3 CMS project.
@@ -23,7 +23,6 @@ use TYPO3\CMS\Core\Http\HtmlResponse;
 use TYPO3\CMS\Core\LinkHandling\LinkService;
 use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Install\FolderStructure\Exception\InvalidArgumentException;
 
 /**
  * Renders the content of a page to be displayed (also in relation to language etc)
@@ -46,13 +45,13 @@ class PageContentErrorHandler implements PageErrorHandlerInterface
      * PageContentErrorHandler constructor.
      * @param int $statusCode
      * @param array $configuration
-     * @throws InvalidArgumentException
+     * @throws \InvalidArgumentException
      */
     public function __construct(int $statusCode, array $configuration)
     {
         $this->statusCode = $statusCode;
         if (empty($configuration['errorContentSource'])) {
-            throw new InvalidArgumentException('PageContentErrorHandler needs to have a proper link set.', 1522826413);
+            throw new \InvalidArgumentException('PageContentErrorHandler needs to have a proper link set.', 1522826413);
         }
         $this->errorHandlerConfiguration = $configuration;
     }
diff --git a/typo3/sysext/frontend/Classes/PageErrorHandler/PageErrorHandlerInterface.php b/typo3/sysext/core/Classes/Error/PageErrorHandler/PageErrorHandlerInterface.php
similarity index 77%
rename from typo3/sysext/frontend/Classes/PageErrorHandler/PageErrorHandlerInterface.php
rename to typo3/sysext/core/Classes/Error/PageErrorHandler/PageErrorHandlerInterface.php
index 82d52a2841a7..29baa6b22758 100644
--- a/typo3/sysext/frontend/Classes/PageErrorHandler/PageErrorHandlerInterface.php
+++ b/typo3/sysext/core/Classes/Error/PageErrorHandler/PageErrorHandlerInterface.php
@@ -1,6 +1,7 @@
 <?php
 declare(strict_types = 1);
-namespace TYPO3\CMS\Frontend\PageErrorHandler;
+
+namespace TYPO3\CMS\Core\Error\PageErrorHandler;
 
 /*
  * This file is part of the TYPO3 CMS project.
@@ -19,8 +20,9 @@ use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 
 /**
- * Page error handler interface
- * Should be implemented by all custom PHP-related Page Error Handlers.
+ * Page error handler interface, used to jump in for Frontend-related calls
+ *
+ * Needs to be implemented by all custom PHP-related Page Error Handlers.
  */
 interface PageErrorHandlerInterface
 {
diff --git a/typo3/sysext/core/Classes/Site/Entity/Site.php b/typo3/sysext/core/Classes/Site/Entity/Site.php
index 36205a03dba1..5e55433209f6 100644
--- a/typo3/sysext/core/Classes/Site/Entity/Site.php
+++ b/typo3/sysext/core/Classes/Site/Entity/Site.php
@@ -16,10 +16,11 @@ namespace TYPO3\CMS\Core\Site\Entity;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Error\PageErrorHandler\FluidPageErrorHandler;
+use TYPO3\CMS\Core\Error\PageErrorHandler\InvalidPageErrorHandlerException;
+use TYPO3\CMS\Core\Error\PageErrorHandler\PageContentErrorHandler;
+use TYPO3\CMS\Core\Error\PageErrorHandler\PageErrorHandlerInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Frontend\PageErrorHandler\FluidPageErrorHandler;
-use TYPO3\CMS\Frontend\PageErrorHandler\PageContentErrorHandler;
-use TYPO3\CMS\Frontend\PageErrorHandler\PageErrorHandlerInterface;
 
 /**
  * Entity representing a single site with available languages
@@ -171,27 +172,28 @@ class Site
     /**
      * Returns a ready-to-use error handler, to be used within the ErrorController
      *
-     * @param int $type
+     * @param int $statusCode
      * @return PageErrorHandlerInterface
      * @throws \RuntimeException
+     * @throws InvalidPageErrorHandlerException
      */
-    public function getErrorHandler(int $type): PageErrorHandlerInterface
+    public function getErrorHandler(int $statusCode): PageErrorHandlerInterface
     {
-        $errorHandler = $this->errorHandlers[$type];
-        switch ($errorHandler['errorHandler']) {
+        $errorHandlerConfiguration = $this->errorHandlers[$statusCode] ?? null;
+        switch ($errorHandlerConfiguration['errorHandler']) {
             case self::ERRORHANDLER_TYPE_FLUID:
-                return new FluidPageErrorHandler($type, $errorHandler);
+                return GeneralUtility::makeInstance(FluidPageErrorHandler::class, $statusCode, $errorHandlerConfiguration);
             case self::ERRORHANDLER_TYPE_PAGE:
-                return new PageContentErrorHandler($type, $errorHandler);
+                return GeneralUtility::makeInstance(PageContentErrorHandler::class, $statusCode, $errorHandlerConfiguration);
             case self::ERRORHANDLER_TYPE_PHP:
+                $handler = GeneralUtility::makeInstance($errorHandlerConfiguration['errorPhpClassFQCN'], $statusCode, $errorHandlerConfiguration);
                 // Check if the interface is implemented
-                $handler = GeneralUtility::makeInstance($errorHandler['errorPhpClassFQCN'], $type, $errorHandler);
                 if (!($handler instanceof PageErrorHandlerInterface)) {
-                    // @todo throw new exception
+                    throw new InvalidPageErrorHandlerException('The configured error handler "' . (string)$errorHandlerConfiguration['errorPhpClassFQCN'] . '" for status code ' . $statusCode . ' must implement the PageErrorHandlerInterface.', 1527432330);
                 }
                 return $handler;
         }
-        throw new \RuntimeException('Not implemented', 1522495914);
+        throw new \RuntimeException('No error handler given for the status code "' . $statusCode . '".', 1522495914);
     }
 
     /**
diff --git a/typo3/sysext/core/Tests/Unit/Site/Entity/SiteTest.php b/typo3/sysext/core/Tests/Unit/Site/Entity/SiteTest.php
new file mode 100644
index 000000000000..91a697c1155a
--- /dev/null
+++ b/typo3/sysext/core/Tests/Unit/Site/Entity/SiteTest.php
@@ -0,0 +1,116 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Core\Tests\Unit\Site\Entity;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Backend\Utility\BackendUtility;
+use TYPO3\CMS\Core\Error\PageErrorHandler\FluidPageErrorHandler;
+use TYPO3\CMS\Core\Error\PageErrorHandler\InvalidPageErrorHandlerException;
+use TYPO3\CMS\Core\Error\PageErrorHandler\PageContentErrorHandler;
+use TYPO3\CMS\Core\Error\PageErrorHandler\PageErrorHandlerInterface;
+use TYPO3\CMS\Core\Site\Entity\Site;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+class SiteTest extends UnitTestCase
+{
+    /**
+     * @test
+     */
+    public function getErrorHandlerReturnsConfiguredErrorHandler()
+    {
+        $subject = new Site('aint-misbehaving', 13, [
+            'languages' => [],
+            'errorHandling' => [
+                [
+                    'errorCode' => 123,
+                    'errorHandler' => 'Fluid',
+                ],
+                [
+                    'errorCode' => 124,
+                    'errorContentSource' => 123,
+                    'errorHandler' => 'Page'
+                ],
+                [
+                    'errorCode' => 125,
+                    'errorHandler' => 'PHP',
+                    'errorContentSource' => 123,
+                    'errorPhpClassFQCN' => PageContentErrorHandler::class
+                ]
+            ]
+        ]);
+
+        $fluidProphecy = $this->prophesize(FluidPageErrorHandler::class);
+        GeneralUtility::addInstance(FluidPageErrorHandler::class, $fluidProphecy->reveal());
+
+        $this->assertEquals(true, $subject->getErrorHandler(123) instanceof PageErrorHandlerInterface);
+        $this->assertEquals(true, $subject->getErrorHandler(124) instanceof PageErrorHandlerInterface);
+        $this->assertEquals(true, $subject->getErrorHandler(125) instanceof PageErrorHandlerInterface);
+    }
+
+    /**
+     * @test
+     */
+    public function getErrorHandlerThrowsExceptionOnInvalidErrorHandler()
+    {
+        $this->expectException(InvalidPageErrorHandlerException::class);
+        $this->expectExceptionCode(1527432330);
+        $this->expectExceptionMessage('The configured error handler "' . BackendUtility::class . '" for status code 404 must implement the PageErrorHandlerInterface.');
+        $subject = new Site('aint-misbehaving', 13, [
+            'languages' => [],
+            'errorHandling' => [
+                [
+                    'errorCode' => 404,
+                    'errorHandler' => 'PHP',
+                    'errorPhpClassFQCN' => BackendUtility::class
+                ],
+            ]
+        ]);
+        $subject->getErrorHandler(404);
+    }
+
+    /**
+     * @test
+     */
+    public function getErrorHandlerThrowsExceptionWhenNoErrorHandlerIsConfigured()
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1522495914);
+        $this->expectExceptionMessage('No error handler given for the status code "404".');
+        $subject = new Site('aint-misbehaving', 13, ['languages' => []]);
+        $subject->getErrorHandler(404);
+    }
+
+    /**
+     * @test
+     */
+    public function getErrorHandlerThrowsExceptionWhenNoErrorHandlerForStatusCodeIsConfigured()
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1522495914);
+        $this->expectExceptionMessage('No error handler given for the status code "404".');
+        $subject = new Site('aint-misbehaving', 13, [
+            'languages' => [],
+            'errorHandling' => [
+                [
+                    'errorCode' => 403,
+                    'errorHandler' => 'Does it really matter?'
+                ],
+            ]
+        ]);
+        $subject->getErrorHandler(404);
+    }
+}
-- 
GitLab