From 7b88ab16af4b7bce73fa85aa604f3a8883e0449f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 3 Jun 2023 17:01:51 -0300 Subject: [PATCH] Fix a case where a fatal error message was not displayed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A fatal error message was not displayed when the ResponseRenderer object already had some HTML, because the output buffer is only displayed when there is no HTML. Signed-off-by: Maurício Meneghini Fauth --- ChangeLog | 1 + libraries/classes/ErrorHandler.php | 39 ++++------------- test/classes/ErrorHandlerTest.php | 77 ++++++++++++++++++++++++++++++++- test/classes/Stubs/ResponseRenderer.php | 13 ++++++ 4 files changed, 97 insertions(+), 33 deletions(-) diff --git a/ChangeLog b/ChangeLog index d825ab92b8..b74c4c5806 100644 --- a/ChangeLog +++ b/ChangeLog @@ -44,6 +44,7 @@ phpMyAdmin - ChangeLog - issue #18441 Fix execute routine page not working when not in a modal - issue #18471 Fix SQL statement not being displayed correctly on RTL languages - issue Fix state times not getting summed in the profiling table +- issue Fix a case where a fatal error message was not displayed 5.2.1 (2023-02-07) - issue #17522 Fix case where the routes cache file is invalid diff --git a/libraries/classes/ErrorHandler.php b/libraries/classes/ErrorHandler.php index 1e114ccc2f..c2c044675b 100644 --- a/libraries/classes/ErrorHandler.php +++ b/libraries/classes/ErrorHandler.php @@ -13,7 +13,6 @@ use function count; use function defined; use function error_reporting; use function get_class; -use function headers_sent; use function htmlspecialchars; use function set_error_handler; use function set_exception_handler; @@ -333,12 +332,16 @@ class ErrorHandler */ protected function dispFatalError(Error $error): void { - if (! headers_sent()) { - $this->dispPageStart($error); + $response = ResponseRenderer::getInstance(); + if (! $response->headersSent()) { + $response->disable(); + $response->addHTML(''); + $response->addHTML($error->getTitle()); + $response->addHTML('' . "\n"); } - echo $error->getDisplay(); - $this->dispPageEnd(); + $response->addHTML($error->getDisplay()); + $response->addHTML(''); if (! defined('TESTSUITE')) { exit; } @@ -370,32 +373,6 @@ class ErrorHandler } /** - * display HTML header - * - * @param Error $error the error - */ - protected function dispPageStart(?Error $error = null): void - { - ResponseRenderer::getInstance()->disable(); - echo ''; - if ($error) { - echo $error->getTitle(); - } else { - echo 'phpMyAdmin error reporting page'; - } - - echo ''; - } - - /** - * display HTML footer - */ - protected function dispPageEnd(): void - { - echo ''; - } - - /** * renders errors not displayed */ public function getDispErrors(): string diff --git a/test/classes/ErrorHandlerTest.php b/test/classes/ErrorHandlerTest.php index c540338ac6..09ddce5d1b 100644 --- a/test/classes/ErrorHandlerTest.php +++ b/test/classes/ErrorHandlerTest.php @@ -7,11 +7,15 @@ namespace PhpMyAdmin\Tests; use Exception; use PhpMyAdmin\Error; use PhpMyAdmin\ErrorHandler; +use PhpMyAdmin\ResponseRenderer; +use PhpMyAdmin\Tests\Stubs\ResponseRenderer as ResponseRendererStub; +use ReflectionProperty; use function array_keys; use function array_pop; use function count; +use const E_ERROR; use const E_RECOVERABLE_ERROR; use const E_USER_NOTICE; use const E_USER_WARNING; @@ -310,11 +314,19 @@ class ErrorHandlerTest extends AbstractTestCase public function testHandleExceptionForDevEnv(): void { + $GLOBALS['lang'] = 'en'; + $GLOBALS['text_dir'] = 'ltr'; + $GLOBALS['PMA_PHP_SELF'] = 'index.php'; $GLOBALS['config']->set('environment', 'development'); + $responseStub = new ResponseRendererStub(); + $property = new ReflectionProperty(ResponseRenderer::class, 'instance'); + $property->setAccessible(true); + $property->setValue($responseStub); + $responseStub->setHeadersSent(true); $errorHandler = new ErrorHandler(); $this->assertSame([], $errorHandler->getCurrentErrors()); $errorHandler->handleException(new Exception('Exception message.')); - $output = $this->getActualOutputForAssertion(); + $output = $responseStub->getHTMLResult(); $errors = $errorHandler->getCurrentErrors(); $this->assertCount(1, $errors); $error = array_pop($errors); @@ -328,11 +340,19 @@ class ErrorHandlerTest extends AbstractTestCase public function testHandleExceptionForProdEnv(): void { + $GLOBALS['lang'] = 'en'; + $GLOBALS['text_dir'] = 'ltr'; + $GLOBALS['PMA_PHP_SELF'] = 'index.php'; $GLOBALS['config']->set('environment', 'production'); + $responseStub = new ResponseRendererStub(); + $property = new ReflectionProperty(ResponseRenderer::class, 'instance'); + $property->setAccessible(true); + $property->setValue($responseStub); + $responseStub->setHeadersSent(true); $errorHandler = new ErrorHandler(); $this->assertSame([], $errorHandler->getCurrentErrors()); $errorHandler->handleException(new Exception('Exception message.')); - $output = $this->getActualOutputForAssertion(); + $output = $responseStub->getHTMLResult(); $errors = $errorHandler->getCurrentErrors(); $this->assertCount(1, $errors); $error = array_pop($errors); @@ -343,4 +363,57 @@ class ErrorHandlerTest extends AbstractTestCase $this->assertStringNotContainsString('Internal error', $output); $this->assertStringNotContainsString('ErrorHandlerTest.php#' . $error->getLine(), $output); } + + public function testAddErrorWithFatalErrorAndHeadersSent(): void + { + $GLOBALS['lang'] = 'en'; + $GLOBALS['text_dir'] = 'ltr'; + $GLOBALS['PMA_PHP_SELF'] = 'index.php'; + $GLOBALS['config']->set('environment', 'production'); + $responseStub = new ResponseRendererStub(); + $property = new ReflectionProperty(ResponseRenderer::class, 'instance'); + $property->setAccessible(true); + $property->setValue($responseStub); + $responseStub->setHeadersSent(true); + $errorHandler = new ErrorHandler(); + $errorHandler->addError('Fatal error message!', E_ERROR, './file/name', 1); + $expectedStart = <<<'HTML' +', $output); + } + + public function testAddErrorWithFatalErrorAndHeadersNotSent(): void + { + $GLOBALS['lang'] = 'en'; + $GLOBALS['text_dir'] = 'ltr'; + $GLOBALS['PMA_PHP_SELF'] = 'index.php'; + $GLOBALS['config']->set('environment', 'production'); + $responseStub = new ResponseRendererStub(); + $property = new ReflectionProperty(ResponseRenderer::class, 'instance'); + $property->setAccessible(true); + $property->setValue($responseStub); + $responseStub->setHeadersSent(false); + $errorHandler = new ErrorHandler(); + $errorHandler->addError('Fatal error message!', E_ERROR, './file/name', 1); + $expectedStart = <<<'HTML' +Error: Fatal error message! +', $output); + } } diff --git a/test/classes/Stubs/ResponseRenderer.php b/test/classes/Stubs/ResponseRenderer.php index 8f1c9a43f5..c1e848d238 100644 --- a/test/classes/Stubs/ResponseRenderer.php +++ b/test/classes/Stubs/ResponseRenderer.php @@ -37,6 +37,9 @@ class ResponseRenderer extends \PhpMyAdmin\ResponseRenderer /** @var int */ private $responseCode = 200; + /** @var bool */ + private $isHeadersSent = false; + /** * Creates a new class instance */ @@ -170,4 +173,14 @@ class ResponseRenderer extends \PhpMyAdmin\ResponseRenderer { return $this->responseCode; } + + public function headersSent(): bool + { + return $this->isHeadersSent; + } + + public function setHeadersSent(bool $isHeadersSent): void + { + $this->isHeadersSent = $isHeadersSent; + } } -- 2.11.4.GIT