diff --git a/lib/Horde/ErrorHandler.php b/lib/Horde/ErrorHandler.php index 43e22a36..ce264a32 100644 --- a/lib/Horde/ErrorHandler.php +++ b/lib/Horde/ErrorHandler.php @@ -87,57 +87,19 @@ public static function fatal($error) if (!headers_sent()) { header('Content-type: text/html; charset=UTF-8'); + header('HTTP/1.1 500: Internal Server Error'); } - echo <<< HTML - -Horde :: Fatal Error - -HTML; - - ob_start(); + try { $admin = (isset($registry) && $registry->isAdmin()); - - echo '

' . Horde_Core_Translation::t("A fatal error has occurred") . '

'; - - if (is_object($error) && method_exists($error, 'getMessage')) { - echo '

' . htmlspecialchars($error->getMessage()) . '

'; - } elseif (is_string($error)) { - echo '

' . htmlspecialchars($error) . '

'; - } - - if ($admin) { - if ($error instanceof Throwable || - $error instanceof Exception) { - $trace = $error; - $file = $error->getFile(); - $line = $error->getLine(); - } else { - $trace = debug_backtrace(); - $calling = array_shift($trace); - $file = $calling['file']; - $line = $calling['line']; - } - printf(Horde_Core_Translation::t("in %s:%d"), $file, $line); - echo '
' .
-                    strval(new Horde_Support_Backtrace($trace)) .
-                    '
'; - if (is_object($error)) { - echo '

' . Horde_Core_Translation::t("Details") . '

'; - echo '

' . Horde_Core_Translation::t("The full error message is logged in Horde's log file, and is shown below only to administrators. Non-administrative users will not see error details.") . '

'; - ob_flush(); - flush(); - echo '
' . htmlspecialchars(print_r($error, true)) . '
'; - } - } else { - echo '

' . Horde_Core_Translation::t("Details have been logged for the administrator.") . '

'; - } + $errorHtml = self::getHtmlForError($error, $admin); } catch (Exception $e) { die($e); } + ob_start(); + echo $errorHtml; ob_end_flush(); - echo ''; exit(1); } @@ -209,4 +171,85 @@ public static function catchFatalError() } } + /** + * Returns html for an error + * + * @param string|Throwable $error The error as string message or Throwable + * @param bool $isAdmin If true will also output the complete trace + * If $error is a Throwable its complete content will also be included in the output + * @return string The full html page for that error as a string + */ + public static function getHtmlForError($error, bool $isAdmin = false): string + { + if ($error instanceof Throwable) { + $message = htmlspecialchars($error->getMessage()); + + } else { + $message = $error; + } + $fatalErrorHasOccoured = Horde_Core_Translation::t('A fatal error has occurred'); + if ($isAdmin){ + $detailsHtml = self::getErrorDetailsAsHtml($error); + } else { + $detailsHtml = '

' . Horde_Core_Translation::t("Details have been logged for the administrator.") . '

'; + } + return << + + Horde :: Fatal Error + + + +

{$fatalErrorHasOccoured}

+

{$message}

+ {$detailsHtml} + + + HTMLDELIM; + + } + + /** + * Get the details of an error as html. This should usually only be output to admin users + * + * @param string|Throwable $error The error as string message or Throwable + * @return string The details of that error as html + */ + protected static function getErrorDetailsAsHtml($error): string + { + if ($error instanceof Throwable) { + $trace = strval(new Horde_Support_Backtrace($error)); + $fileName = $error->getFile(); + $lineNo = $error->getLine(); + + $translateDetails = Horde_Core_Translation::t('Details'); + $translateLogInfo = Horde_Core_Translation::t('The full error message is logged in Horde\'s log file, and is shown below only to administrators. Non-administrative users will not see error details.'); + $fullErrorAsHtml = htmlspecialchars(print_r($error, true)); + $fullDetailsHtml = <<$translateDetails +

{$translateLogInfo}

+
+ 
$fullErrorAsHtml
+
+ HTMLDELIM; + } else { + $trace = debug_backtrace(); + $calling = array_shift($trace); + $trace = strval(new Horde_Support_Backtrace($trace)); + $fileName = $calling['file']; + $lineNo = $calling['line']; + $fullDetailsHtml = ''; + } + $translateIn = sprintf(Horde_Core_Translation::t('in %s:%d'), $fileName, $lineNo); + + return << + 
+            {$trace}
+
+ + {$fullDetailsHtml} + HTMLDELIM; + } } diff --git a/src/Middleware/AppFinder.php b/src/Middleware/AppFinder.php index 55ddfd0e..5fdada1e 100644 --- a/src/Middleware/AppFinder.php +++ b/src/Middleware/AppFinder.php @@ -6,6 +6,8 @@ use Exception; use Horde; +use Horde\Http\ResponseFactory; +use Horde\Http\StreamFactory; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Server\MiddlewareInterface; @@ -31,9 +33,17 @@ class AppFinder implements MiddlewareInterface { private Horde_Registry $registry; - public function __construct(Horde_Registry $registry) - { + private ResponseFactory $responseFactory; + private StreamFactory $streamFactory; + + public function __construct( + Horde_Registry $registry, + ResponseFactory $responseFactory, + StreamFactory $streamFactory + ) { $this->registry = $registry; + $this->responseFactory = $responseFactory; + $this->streamFactory = $streamFactory; } /** * Rebuild a path string to a common form @@ -171,7 +181,12 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface // If we still found no app, give up if (empty($found)) { $path = $request->getUri()->getPath(); - throw new \Exception("No App found for path: $path"); + $msg = sprintf('No App found for path: %s', $path); + Horde::log($msg, 'INFO'); + return $this->responseFactory->createResponse( + 404, + 'Not Found' + )->withBody($this->streamFactory->createStream($msg)); } $prefix = $found['path']; diff --git a/src/Middleware/ErrorFilter.php b/src/Middleware/ErrorFilter.php index bf641d0b..dfcf2662 100644 --- a/src/Middleware/ErrorFilter.php +++ b/src/Middleware/ErrorFilter.php @@ -4,12 +4,16 @@ namespace Horde\Core\Middleware; +use Horde; +use Horde\Http\ResponseFactory; +use Horde\Http\StreamFactory; +use Horde_ErrorHandler; +use Horde_Registry; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Horde_Registry; -use Horde_Application; +use Throwable; /** * ErrorFilter middleware @@ -31,9 +35,58 @@ */ class ErrorFilter implements MiddlewareInterface { + protected Horde_Registry $registry; + protected ResponseFactory $responseFactory; + protected StreamFactory $streamFactory; + + public function __construct( + Horde_Registry $registry, + ResponseFactory $responseFactory, + StreamFactory $streamFactory + ) { + $this->registry = $registry; + $this->responseFactory = $responseFactory; + $this->streamFactory = $streamFactory; + } + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - // TODO: - return $handler->handle($request); + try { + return $handler->handle($request); + } catch (Throwable $throwable) { + Horde::log($throwable, 'EMERG'); + return $this->getErrorResponse($request, $throwable); + } + } + + protected function getErrorResponse(ServerRequestInterface $request, Throwable $throwable): ResponseInterface + { + $isAdmin = $this->registry->isAdmin(); + $acceptsJson = in_array('application/json', array_map(fn($val) => strtolower($val), $request->getHeader('Accept'))); + if ($acceptsJson){ + return $this->getJsonResponse($throwable, $isAdmin); + } else { + return $this->getHtmlResponse($throwable, $isAdmin); + } + } + + protected function getJsonResponse(Throwable $throwable, bool $isAdmin = false): ResponseInterface + { + $json = json_encode([ + 'message' => $throwable->getMessage(), + 'code' => $throwable->getCode(), + 'trace' => $isAdmin ? $throwable->getTrace() : [], + ]); + $stream = $this->streamFactory->createStream($json); + return $this->responseFactory->createResponse(500, 'Internal Server Error') + ->withBody($stream) + ->withHeader('Content-Type', 'application/json'); + } + + protected function getHtmlResponse(Throwable $throwable, bool $isAdmin = false): ResponseInterface + { + $stream = $this->streamFactory->createStream(Horde_ErrorHandler::getHtmlForError($throwable, $isAdmin)); + return $this->responseFactory->createResponse(500, 'Internal Server Error') + ->withBody($stream); } } diff --git a/src/Middleware/HordeCore.php b/src/Middleware/HordeCore.php index 7b19ac1d..8ea22e6b 100644 --- a/src/Middleware/HordeCore.php +++ b/src/Middleware/HordeCore.php @@ -56,13 +56,15 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface $injector->setInstance(ResponseFactoryInterface::class, new ResponseFactory()); } - - // Detect correct app $registry = $injector->getInstance('Horde_Registry'); - $request = $request->withAttribute('registry', $registry); - $handler->addMiddleware(new AppFinder($registry)); + // First middleware should be ErrorFilter to catch all errors + $handler->addMiddleware(new ErrorFilter($registry, new ResponseFactory(), new StreamFactory())); + // Detect correct app + $handler->addMiddleware(new AppFinder($registry, new ResponseFactory(), new StreamFactory())); // Find route inside detected app $handler->addMiddleware(new AppRouter($registry, $injector->get('Horde_Routes_Mapper'), $injector)); + + $request = $request->withAttribute('registry', $registry); return $handler->handle($request); } } diff --git a/test/Middleware/AppFinderTest.php b/test/Middleware/AppFinderTest.php index aee1f19d..70fe8035 100644 --- a/test/Middleware/AppFinderTest.php +++ b/test/Middleware/AppFinderTest.php @@ -15,7 +15,8 @@ use Exception; use Horde\Core\Middleware\AppFinder; - +use Horde\Http\ResponseFactory; +use Horde\Http\StreamFactory; use Horde\Test\TestCase; use Horde_Registry; @@ -27,7 +28,9 @@ class AppFinderTest extends TestCase protected function getMiddleware() { return new AppFinder( - $this->registry + $this->registry, + new ResponseFactory(), + new StreamFactory() ); } @@ -86,8 +89,9 @@ public function testNoValidAppInPath() }); $middleware = $this->getMiddleware(); - $this->expectException(Exception::class); - $middleware->process($request, $this->handler); + //$this->expectException(Exception::class); + $response = $middleware->process($request, $this->handler); + $this->assertSame(404, $response->getStatusCode()); } /** @@ -137,8 +141,8 @@ public function testNoAppAvailable() }); $middleware = $this->getMiddleware(); - $this->expectException(Exception::class); - $middleware->process($request, $this->handler); + $response = $middleware->process($request, $this->handler); + $this->assertSame(404, $response->getStatusCode()); } /** @@ -188,8 +192,8 @@ public function testDifferntScheme() $middleware = $this->getMiddleware(); - $this->expectException(Exception::class); - $middleware->process($request, $this->handler); + $response = $middleware->process($request, $this->handler); + $this->assertSame(404, $response->getStatusCode()); } /** @@ -213,8 +217,8 @@ public function testDifferentHost() $middleware = $this->getMiddleware(); - $this->expectException(Exception::class); - $middleware->process($request, $this->handler); + $response = $middleware->process($request, $this->handler); + $this->assertSame(404, $response->getStatusCode()); } @@ -236,8 +240,8 @@ public function testEmptyPath() }); $middleware = $this->getMiddleware(); - $this->expectException(Exception::class); - $middleware->process($request, $this->handler); + $response = $middleware->process($request, $this->handler); + $this->assertSame(404, $response->getStatusCode()); } public function testFindAppBehindDifferentApp() @@ -333,7 +337,7 @@ public function testDoNotFindWithoutAlias() }); $middleware = $this->getMiddleware(); - $this->expectException(Exception::class); - $middleware->process($request, $this->handler); + $response = $middleware->process($request, $this->handler); + $this->assertSame(404, $response->getStatusCode()); } }