Merge pull request #1553 from ArthurHoaro/fix/404-page

Properly handle 404 errors

diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php
index 58067c9945319d7d7bb664d37c0589fe71b8d377..55bb51b5b46506f95b2d79280796c694cc1e9fc9 100644 (file)
--- 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 9a9a974a6785bce8077e818607dc75e5dbee7e63..66e669aaed3fd6760c966d0fbfd1689696c74f20 100644 (file)
--- 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 (file)
index 0000000..758dd83
--- /dev/null
+++ b/application/front/controller/visitor/ErrorNotFoundController.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Visitor;
+
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+/**
+ * Controller used to render the 404 error page.
+ */
+class ErrorNotFoundController extends ShaarliVisitorController
+{
+    public function __invoke(Request $request, Response $response): Response
+    {
+        // Request from the API
+        if (false !== strpos($request->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 c08010ae915a778e161a424d0ead26a097b20d5e..2047a63adf1bd6143609d854b5d47b44669ea55d 100644 (file)
--- 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 (file)
index 0000000..625467b
--- /dev/null
+++ b/tests/front/controller/visitor/ErrorNotFoundControllerTest.php
@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Visitor;
+
+use PHPUnit\Framework\TestCase;
+use Slim\Http\Request;
+use Slim\Http\Response;
+use Slim\Http\Uri;
+
+class ErrorNotFoundControllerTest extends TestCase
+{
+    use FrontControllerMockHelper;
+
+    /** @var ErrorNotFoundController */
+    protected $controller;
+
+    public function setUp(): void
+    {
+        $this->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 e0bd4ecf5d14cd426546caf94893873f75c55fc2..927e7f0a15eef7acda822afc5ef1eaf6322e3571 100644 (file)
--- 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;