Commit 759e8daa authored by Benjamin Franzke's avatar Benjamin Franzke Committed by Christian Kuhn
Browse files

[BUGFIX] Streamline frontend 5xx error handling

 * Fix 500 vs 503 error for configuration vs maintenance errors
 * Fix maintenance mode middleware ordering to get access to site
   related error handlers
 * Fix ErrorController unit tests to actually test
   for 503 and 500 error responses (contained duplicate
   code that didn't actually test for the non-configured
   state, but the devIPMask state)

For 5xx status code we have two different cases right now:
 * configuration errors, which need to respond with 500
 * maintenance mode, which is a 503 response

Therefore TSFE now uses internalErrorAction()
to respond with "500 Internal Server Error" when
the page configuration is broken.

Resolves: #93045
Related: #93032
Releases: master, 10.4
Change-Id: I60de9f7ba06d17f2e6e5c8f20c9fc10e90b4175b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/67080

Tested-by: default avatarMartin Kutschker <mkutschker-typo3@yahoo.com>
Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: default avatarMartin Kutschker <mkutschker-typo3@yahoo.com>
Reviewed-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
parent d94b1b5b
<?php
/*
* 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!
*/
namespace TYPO3\CMS\Core\Error\Http;
use TYPO3\CMS\Core\Utility\HttpUtility;
/**
* Exception for Error 500 - Internal Server Error
*/
class InternalServerErrorException extends AbstractServerErrorException
{
/**
* @var array HTTP Status Header lines
*/
protected $statusHeaders = [HttpUtility::HTTP_STATUS_500];
/**
* @var string Title of the message
*/
protected $title = 'Internal Server Error (500)';
/**
* @var string Error Message
*/
protected $message = 'This page is currently not available due to server errors.';
/**
* Constructor for this Status Exception
*
* @param string $message Error Message
* @param int $code Exception Code
*/
public function __construct($message = null, $code = 0)
{
if (!empty($message)) {
$this->message = $message;
}
parent::__construct($this->statusHeaders, $this->message, $this->title, $code);
}
}
......@@ -20,6 +20,7 @@ namespace TYPO3\CMS\Frontend\Controller;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use TYPO3\CMS\Core\Controller\ErrorPageController;
use TYPO3\CMS\Core\Error\Http\InternalServerErrorException;
use TYPO3\CMS\Core\Error\Http\PageNotFoundException;
use TYPO3\CMS\Core\Error\Http\ServiceUnavailableException;
use TYPO3\CMS\Core\Error\PageErrorHandler\PageErrorHandlerInterface;
......@@ -30,19 +31,41 @@ use TYPO3\CMS\Core\Site\Entity\Site;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
* Handles "Page Not Found" or "Page Unavailable" requests,
* Handles error requests,
* returns a response object.
*/
class ErrorController
{
/**
* Used for creating a 503 response ("Page unavailable"), usually due some misconfiguration
* Used for creating a 500 response ("Internal Server Error"), usually due some misconfiguration
* but if configured, a RedirectResponse could be returned as well.
*
* @param ServerRequestInterface $request
* @param string $message
* @param array $reasons
* @return ResponseInterface
* @throws InternalServerErrorException
*/
public function internalErrorAction(ServerRequestInterface $request, string $message, array $reasons = []): ResponseInterface
{
if (!$this->isPageUnavailableHandlerConfigured()) {
throw new InternalServerErrorException($message, 1607585445);
}
$errorHandler = $this->getErrorHandlerFromSite($request, 500);
if ($errorHandler instanceof PageErrorHandlerInterface) {
return $errorHandler->handlePageError($request, $message, $reasons);
}
return $this->handleDefaultError($request, 500, $message ?: 'Internal Server Error');
}
/**
* Used for creating a 503 response ("Service Unavailable"), to be used for maintenance mode
* or when the server is overloaded, a RedirectResponse could be returned as well.
*
* @param ServerRequestInterface $request
* @param string $message
* @param array $reasons
* @return ResponseInterface
* @throws ServiceUnavailableException
*/
public function unavailableAction(ServerRequestInterface $request, string $message, array $reasons = []): ResponseInterface
......@@ -54,7 +77,7 @@ class ErrorController
if ($errorHandler instanceof PageErrorHandlerInterface) {
return $errorHandler->handlePageError($request, $message, $reasons);
}
return $this->handleDefaultError($request, 503, $message ?: 'Page is unavailable');
return $this->handleDefaultError($request, 503, $message ?: 'Service Unavailable');
}
/**
......@@ -104,10 +127,10 @@ class ErrorController
}
/**
* Checks whether the pageUnavailableHandler should be used. To be used, pageUnavailable_handling must be set
* and devIPMask must not match the current visitor's IP address.
* Checks whether the devIPMask matches the current visitor's IP address.
* Note: the name of this method is a misnomer (legacy code),
*
* @return bool TRUE/FALSE whether the pageUnavailable_handler should be used.
* @return bool True if the server error handler should be used.
*/
protected function isPageUnavailableHandlerConfigured(): bool
{
......
......@@ -39,8 +39,8 @@ use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\EndTimeRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\StartTimeRestriction;
use TYPO3\CMS\Core\Domain\Repository\PageRepository;
use TYPO3\CMS\Core\Error\Http\AbstractServerErrorException;
use TYPO3\CMS\Core\Error\Http\PageNotFoundException;
use TYPO3\CMS\Core\Error\Http\ServiceUnavailableException;
use TYPO3\CMS\Core\Error\Http\ShortcutTargetPageNotFoundException;
use TYPO3\CMS\Core\Exception\Page\RootLineException;
use TYPO3\CMS\Core\Http\ApplicationType;
......@@ -1045,7 +1045,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
* @see TypoScriptFrontendController::$originalMountPointPage
* @see TypoScriptFrontendController::$originalShortcutPage
*
* @throws ServiceUnavailableException
* @throws \TYPO3\CMS\Core\Error\Http\ServiceUnavailableException
* @throws PageNotFoundException
*/
protected function getPageAndRootline(ServerRequestInterface $request)
......@@ -1161,14 +1161,16 @@ class TypoScriptFrontendController implements LoggerAwareInterface
$message = 'The requested page didn\'t have a proper connection to the tree-root!';
$this->logger->error($message);
try {
$response = GeneralUtility::makeInstance(ErrorController::class)->unavailableAction(
$response = GeneralUtility::makeInstance(ErrorController::class)->internalErrorAction(
$request,
$message,
$this->getPageAccessFailureReasons(PageAccessFailureReasons::ROOTLINE_BROKEN)
);
throw new ImmediateResponseException($response, 1533931350);
} catch (ServiceUnavailableException $e) {
throw new ServiceUnavailableException($message, 1301648167);
} catch (AbstractServerErrorException $e) {
$this->logger->error($message);
$exceptionClass = get_class($e);
throw new $exceptionClass($message, 1301648167);
}
}
// Checking for include section regarding the hidden/starttime/endtime/fe_user (that is access control of a whole subbranch!)
......@@ -1176,15 +1178,16 @@ class TypoScriptFrontendController implements LoggerAwareInterface
if (empty($this->rootLine)) {
$message = 'The requested page was not accessible!';
try {
$response = GeneralUtility::makeInstance(ErrorController::class)->unavailableAction(
$response = GeneralUtility::makeInstance(ErrorController::class)->internalErrorAction(
$request,
$message,
$this->getPageAccessFailureReasons(PageAccessFailureReasons::ACCESS_DENIED_GENERAL)
);
throw new ImmediateResponseException($response, 1533931351);
} catch (ServiceUnavailableException $e) {
} catch (AbstractServerErrorException $e) {
$this->logger->warning($message);
throw new ServiceUnavailableException($message, 1301648234);
$exceptionClass = get_class($e);
throw new $exceptionClass($message, 1301648234);
}
} else {
$el = reset($this->rootLine);
......@@ -1742,7 +1745,8 @@ class TypoScriptFrontendController implements LoggerAwareInterface
* Checks if config-array exists already but if not, gets it
*
* @param ServerRequestInterface|null $request
* @throws ServiceUnavailableException
* @throws \TYPO3\CMS\Core\Error\Http\InternalServerErrorException
* @throws \TYPO3\CMS\Core\Error\Http\ServiceUnavailableException
*/
public function getConfigArray(ServerRequestInterface $request = null)
{
......@@ -1771,15 +1775,16 @@ class TypoScriptFrontendController implements LoggerAwareInterface
$message = 'The page is not configured! [type=' . $this->type . '][' . $typoScriptPageTypeName . '].';
$this->logger->alert($message);
try {
$response = GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
$response = GeneralUtility::makeInstance(ErrorController::class)->internalErrorAction(
$request,
$message,
['code' => PageAccessFailureReasons::RENDERING_INSTRUCTIONS_NOT_CONFIGURED]
);
throw new ImmediateResponseException($response, 1533931374);
} catch (PageNotFoundException $e) {
} catch (AbstractServerErrorException $e) {
$explanation = 'This means that there is no TypoScript object of type PAGE with typeNum=' . $this->type . ' configured.';
throw new ServiceUnavailableException($message . ' ' . $explanation, 1294587217);
$exceptionClass = get_class($e);
throw new $exceptionClass($message . ' ' . $explanation, 1294587217);
}
} else {
if (!isset($this->config['config'])) {
......@@ -1832,14 +1837,15 @@ class TypoScriptFrontendController implements LoggerAwareInterface
$message = 'No TypoScript template found!';
$this->logger->alert($message);
try {
$response = GeneralUtility::makeInstance(ErrorController::class)->unavailableAction(
$response = GeneralUtility::makeInstance(ErrorController::class)->internalErrorAction(
$request,
$message,
['code' => PageAccessFailureReasons::RENDERING_INSTRUCTIONS_NOT_FOUND]
);
throw new ImmediateResponseException($response, 1533931380);
} catch (ServiceUnavailableException $e) {
throw new ServiceUnavailableException($message, 1294587218);
} catch (AbstractServerErrorException $e) {
$exceptionClass = get_class($e);
throw new $exceptionClass($message, 1294587218);
}
}
}
......
......@@ -33,6 +33,7 @@ return [
'target' => \TYPO3\CMS\Frontend\Middleware\MaintenanceMode::class,
'after' => [
'typo3/cms-core/normalized-params-attribute',
'typo3/cms-frontend/site',
'typo3/cms-frontend/eid'
]
],
......
......@@ -299,7 +299,7 @@ class SlugSiteRequestTest extends AbstractTestCase
/**
* @test
*/
public function unconfiguredTypeNumResultsIn404Error()
public function unconfiguredTypeNumResultsIn500Error()
{
$this->writeSiteConfiguration(
'website-local',
......@@ -307,7 +307,7 @@ class SlugSiteRequestTest extends AbstractTestCase
[
$this->buildDefaultLanguageConfiguration('EN', '/en-en/')
],
$this->buildErrorHandlingConfiguration('Fluid', [404])
$this->buildErrorHandlingConfiguration('Fluid', [500])
);
$uri = 'https://website.local/en-en/?type=13';
......@@ -317,7 +317,7 @@ class SlugSiteRequestTest extends AbstractTestCase
);
self::assertSame(
404,
500,
$response->getStatusCode()
);
self::assertStringContainsString(
......
......@@ -33,7 +33,7 @@ class ErrorControllerTest extends UnitTestCase
/**
* @test
*/
public function pageNotFoundHandlingThrowsExceptionIfNotConfigured()
public function pageNotFoundHandlingReturns404ResponseIfNotConfigured()
{
$typo3InformationProphecy = $this->prophesize(Typo3Information::class);
$typo3InformationProphecy->getCopyrightYear()->willReturn('1999-20XX');
......@@ -48,10 +48,26 @@ class ErrorControllerTest extends UnitTestCase
/**
* @test
*/
public function unavailableHandlingThrowsExceptionIfNotConfigured()
public function unavailableHandlingReturns503ResponseIfNotConfigured()
{
$typo3InformationProphecy = $this->prophesize(Typo3Information::class);
$typo3InformationProphecy->getCopyrightYear()->willReturn('1999-20XX');
GeneralUtility::addInstance(Typo3Information::class, $typo3InformationProphecy->reveal());
$GLOBALS['TYPO3_REQUEST'] = [];
$subject = new ErrorController();
$response = $subject->unavailableAction(new ServerRequest(), 'This page is temporarily unavailable.');
self::assertSame(503, $response->getStatusCode());
self::assertStringContainsString('This page is temporarily unavailable.', $response->getBody()->getContents());
}
/**
* @test
*/
public function unavailableHandlingDoesNotTriggerDueToDevIpMask()
{
$GLOBALS['TYPO3_CONF_VARS']['SYS']['devIPmask'] = '*';
$_SERVER['REMOTE_ADDR'] = '127.0.0.1';
$this->expectExceptionMessage('All your system are belong to us!');
$this->expectExceptionCode(1518472181);
$subject = new ErrorController();
......@@ -61,15 +77,29 @@ class ErrorControllerTest extends UnitTestCase
/**
* @test
*/
public function unavailableHandlingDoesNotTriggerDueToDevIpMask()
public function internalErrorHandlingReturns500ResponseIfNotConfigured()
{
$typo3InformationProphecy = $this->prophesize(Typo3Information::class);
$typo3InformationProphecy->getCopyrightYear()->willReturn('1999-20XX');
GeneralUtility::addInstance(Typo3Information::class, $typo3InformationProphecy->reveal());
$subject = new ErrorController();
$response = $subject->internalErrorAction(new ServerRequest(), 'All your system are belong to us!');
self::assertSame(500, $response->getStatusCode());
self::assertStringContainsString('All your system are belong to us!', $response->getBody()->getContents());
}
/**
* @test
*/
public function internalErrorHandlingDoesNotTriggerDueToDevIpMask()
{
$GLOBALS['TYPO3_CONF_VARS']['SYS']['devIPmask'] = '*';
$_SERVER['REMOTE_ADDR'] = '127.0.0.1';
$this->expectExceptionMessage('All your system are belong to us!');
$this->expectExceptionCode(1518472181);
$this->expectExceptionCode(1607585445);
$subject = new ErrorController();
$subject->unavailableAction(new ServerRequest(), 'All your system are belong to us!');
$subject->internalErrorAction(new ServerRequest(), 'All your system are belong to us!');
}
/**
......@@ -128,6 +158,34 @@ class ErrorControllerTest extends UnitTestCase
self::assertEquals(['reason' => 'Error handler is not configured.'], $responseContent);
}
/**
* @test
*/
public function defaultErrorHandlerWithHtmlResponseIsChosenWhenNoSiteConfiguredForInternalErrorAction()
{
$typo3InformationProphecy = $this->prophesize(Typo3Information::class);
$typo3InformationProphecy->getCopyrightYear()->willReturn('1999-20XX');
GeneralUtility::addInstance(Typo3Information::class, $typo3InformationProphecy->reveal());
$subject = new ErrorController();
$response = $subject->internalErrorAction(new ServerRequest(), 'Error handler is not configured.');
self::assertSame(500, $response->getStatusCode());
self::assertSame('text/html; charset=utf-8', $response->getHeaderLine('Content-Type'));
self::assertStringContainsString('Error handler is not configured.', $response->getBody()->getContents());
}
/**
* @test
*/
public function defaultErrorHandlerWithJsonResponseIsChosenWhenNoSiteConfiguredForInternalErrorAction()
{
$subject = new ErrorController();
$response = $subject->internalErrorAction((new ServerRequest())->withAddedHeader('Accept', 'application/json'), 'Error handler is not configured.');
$responseContent = \json_decode($response->getBody()->getContents(), true);
self::assertSame(500, $response->getStatusCode());
self::assertSame('application/json; charset=utf-8', $response->getHeaderLine('Content-Type'));
self::assertEquals(['reason' => 'Error handler is not configured.'], $responseContent);
}
/**
* @test
*/
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment