From: ArthurHoaro Date: Sat, 12 Sep 2020 10:42:19 +0000 (+0200) Subject: Properly handle 404 errors X-Git-Tag: v0.12.0-beta-1~14^2 X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=d52ab0b1e99aa0c494f389092dce1e926296032d;p=github%2Fshaarli%2FShaarli.git Properly handle 404 errors Use 404 template instead of default Slim error page if the route is not found. Fixes #827 --- diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php index 58067c99..55bb51b5 100644 --- a/application/container/ContainerBuilder.php +++ b/application/container/ContainerBuilder.php @@ -10,6 +10,7 @@ use Shaarli\Config\ConfigManager; use Shaarli\Feed\FeedBuilder; use Shaarli\Formatter\FormatterFactory; use Shaarli\Front\Controller\Visitor\ErrorController; +use Shaarli\Front\Controller\Visitor\ErrorNotFoundController; use Shaarli\History; use Shaarli\Http\HttpAccess; use Shaarli\Netscape\NetscapeBookmarkUtils; @@ -149,6 +150,9 @@ class ContainerBuilder ); }; + $container['notFoundHandler'] = function (ShaarliContainer $container): ErrorNotFoundController { + return new ErrorNotFoundController($container); + }; $container['errorHandler'] = function (ShaarliContainer $container): ErrorController { return new ErrorController($container); }; diff --git a/application/container/ShaarliContainer.php b/application/container/ShaarliContainer.php index 9a9a974a..66e669aa 100644 --- a/application/container/ShaarliContainer.php +++ b/application/container/ShaarliContainer.php @@ -24,21 +24,22 @@ use Slim\Container; /** * Extension of Slim container to document the injected objects. * - * @property string $basePath Shaarli's instance base path (e.g. `/shaarli/`) + * @property string $basePath Shaarli's instance base path (e.g. `/shaarli/`) * @property BookmarkServiceInterface $bookmarkService * @property CookieManager $cookieManager * @property ConfigManager $conf - * @property mixed[] $environment $_SERVER automatically injected by Slim - * @property callable $errorHandler Overrides default Slim exception display + * @property mixed[] $environment $_SERVER automatically injected by Slim + * @property callable $errorHandler Overrides default Slim exception display * @property FeedBuilder $feedBuilder * @property FormatterFactory $formatterFactory * @property History $history * @property HttpAccess $httpAccess * @property LoginManager $loginManager * @property NetscapeBookmarkUtils $netscapeBookmarkUtils + * @property callable $notFoundHandler Overrides default Slim exception display * @property PageBuilder $pageBuilder * @property PageCacheManager $pageCacheManager - * @property callable $phpErrorHandler Overrides default Slim PHP error display + * @property callable $phpErrorHandler Overrides default Slim PHP error display * @property PluginManager $pluginManager * @property SessionManager $sessionManager * @property Thumbnailer $thumbnailer diff --git a/application/front/controller/visitor/ErrorNotFoundController.php b/application/front/controller/visitor/ErrorNotFoundController.php new file mode 100644 index 00000000..758dd83b --- /dev/null +++ b/application/front/controller/visitor/ErrorNotFoundController.php @@ -0,0 +1,29 @@ +getRequestTarget(), '/api/v1')) { + return $response->withStatus(404); + } + + // This is required because the middleware is ignored if the route is not found. + $this->container->basePath = rtrim($request->getUri()->getBasePath(), '/'); + + $this->assignView('error_message', t('Requested page could not be found.')); + + return $response->withStatus(404)->write($this->render('404')); + } +} diff --git a/tests/container/ContainerBuilderTest.php b/tests/container/ContainerBuilderTest.php index c08010ae..2047a63a 100644 --- a/tests/container/ContainerBuilderTest.php +++ b/tests/container/ContainerBuilderTest.php @@ -10,6 +10,7 @@ use Shaarli\Config\ConfigManager; use Shaarli\Feed\FeedBuilder; use Shaarli\Formatter\FormatterFactory; use Shaarli\Front\Controller\Visitor\ErrorController; +use Shaarli\Front\Controller\Visitor\ErrorNotFoundController; use Shaarli\History; use Shaarli\Http\HttpAccess; use Shaarli\Netscape\NetscapeBookmarkUtils; @@ -75,6 +76,7 @@ class ContainerBuilderTest extends TestCase static::assertInstanceOf(PageBuilder::class, $container->pageBuilder); static::assertInstanceOf(PageCacheManager::class, $container->pageCacheManager); static::assertInstanceOf(ErrorController::class, $container->phpErrorHandler); + static::assertInstanceOf(ErrorNotFoundController::class, $container->notFoundHandler); static::assertInstanceOf(PluginManager::class, $container->pluginManager); static::assertInstanceOf(SessionManager::class, $container->sessionManager); static::assertInstanceOf(Thumbnailer::class, $container->thumbnailer); diff --git a/tests/front/controller/visitor/ErrorNotFoundControllerTest.php b/tests/front/controller/visitor/ErrorNotFoundControllerTest.php new file mode 100644 index 00000000..625467b1 --- /dev/null +++ b/tests/front/controller/visitor/ErrorNotFoundControllerTest.php @@ -0,0 +1,81 @@ +createContainer(); + + $this->controller = new ErrorNotFoundController($this->container); + } + + /** + * Test displaying 404 error + */ + public function testDisplayNotFoundError(): void + { + $request = $this->createMock(Request::class); + $request->expects(static::once())->method('getRequestTarget')->willReturn('/'); + $request->method('getUri')->willReturnCallback(function (): Uri { + $uri = $this->createMock(Uri::class); + $uri->method('getBasePath')->willReturn('/subfolder'); + + return $uri; + }); + + $response = new Response(); + + // Save RainTPL assigned variables + $assignedVariables = []; + $this->assignTemplateVars($assignedVariables); + + $result = ($this->controller)( + $request, + $response + ); + + static::assertSame(404, $result->getStatusCode()); + static::assertSame('404', (string) $result->getBody()); + static::assertSame('Requested page could not be found.', $assignedVariables['error_message']); + } + + /** + * Test displaying 404 error from REST API + */ + public function testDisplayNotFoundErrorFromAPI(): void + { + $request = $this->createMock(Request::class); + $request->expects(static::once())->method('getRequestTarget')->willReturn('/sufolder/api/v1/links'); + $request->method('getUri')->willReturnCallback(function (): Uri { + $uri = $this->createMock(Uri::class); + $uri->method('getBasePath')->willReturn('/subfolder'); + + return $uri; + }); + + $response = new Response(); + + // Save RainTPL assigned variables + $assignedVariables = []; + $this->assignTemplateVars($assignedVariables); + + $result = ($this->controller)($request, $response); + + static::assertSame(404, $result->getStatusCode()); + static::assertSame([], $assignedVariables); + } +} diff --git a/tests/front/controller/visitor/FrontControllerMockHelper.php b/tests/front/controller/visitor/FrontControllerMockHelper.php index e0bd4ecf..927e7f0a 100644 --- a/tests/front/controller/visitor/FrontControllerMockHelper.php +++ b/tests/front/controller/visitor/FrontControllerMockHelper.php @@ -94,7 +94,6 @@ trait FrontControllerMockHelper protected function assignTemplateVars(array &$variables): void { $this->container->pageBuilder - ->expects(static::atLeastOnce()) ->method('assign') ->willReturnCallback(function ($key, $value) use (&$variables) { $variables[$key] = $value;