Merge pull request #1477 from bolt/fix/use-correct-error-controller

Use new Symfony Error Controller, instead of Twig's old one.
bolt · Jun 15, 2020 · 6fe95da · 6fe95da
2 parents dc16ef1 + 7f3d5ba
commit 6fe95da
Show file tree

Hide file tree

Showing 13 changed files with 170 additions and 29 deletions.
diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml
@@ -29,3 +29,6 @@ framework:
     validation:
         email_validation_mode: 'html5'
         enable_annotations: true
+
+    # Override Symfony's error controller, so we can show custom 404's and 503's
+    error_controller: Bolt\Controller\ErrorController::showAction
diff --git a/config/packages/twig.yaml b/config/packages/twig.yaml
@@ -12,5 +12,6 @@ twig:
     globals:
         'config': '@Bolt\Configuration\Config'
 
-    # Override Symfony's error pages (so we can show custom 404's)
-    exception_controller: Bolt\Controller\ExceptionController::showAction
+    # Silence Twig / Symfony's exception controller here, so we can set it up properly in
+    # `framework.yaml` (so we can show custom 404's)
+    exception_controller: ~
diff --git a/config/services.yaml b/config/services.yaml
@@ -41,12 +41,6 @@ services:
         resource: '../src/Controller'
         tags: ['controller.service_arguments']
 
-    # Override Symfony's error pages (so we can show custom 404's)
-    Bolt\Controller\ExceptionController:
-        public: true
-        arguments:
-            $debug: '%kernel.debug%'
-
     Bolt\Doctrine\TablePrefix:
         tags:
             - { name: doctrine.event_listener, event: loadClassMetadata, lazy: true }
@@ -110,3 +104,10 @@ services:
 
     Twig\Extension\StringLoaderExtension: ~
 
+    Symfony\Component\ErrorHandler\ErrorRenderer\ErrorRendererInterface: '@error_handler.error_renderer.html'
+
+    # @deprecated since Bolt 4.0.0, use Bolt\Controller\ErrorController instead
+    Bolt\Controller\ExceptionController:
+        public: true
+        arguments:
+            $debug: '%kernel.debug%'
diff --git a/src/Controller/Backend/FilemanagerController.php b/src/Controller/Backend/FilemanagerController.php
@@ -64,7 +64,7 @@ public function filemanager(string $location, Request $request): Response
         $finder = $this->findFiles($location->getBasepath(), $path);
         $folders = $this->findFolders($location->getBasepath(), $path);
 
-        $currentPage = (int) $request->query->get('page', 1);
+        $currentPage = (int) $request->query->get('page', '1');
         $pager = $this->createPaginator($finder, $currentPage);
 
         $parent = $path !== '/' ? Path::canonicalize($path . '/..') : '';

diff --git a/src/Controller/Backend/ListingController.php b/src/Controller/Backend/ListingController.php
@@ -23,7 +23,7 @@ class ListingController extends TwigAwareController implements BackendZoneInterf
     public function overview(Request $request, Query $query, string $contentType = ''): Response
     {
         $contentTypeObject = ContentType::factory($contentType, $this->config->get('contenttypes'));
-        $page = (int) $request->query->get('page', 1);
+        $page = (int) $request->query->get('page', '1');
 
         $params = [
             'status' => '!unknown',

diff --git a/src/Controller/ErrorController.php b/src/Controller/ErrorController.php
@@ -0,0 +1,126 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Bolt\Controller;
+
+use Bolt\Configuration\Config;
+use Bolt\Controller\Frontend\DetailController;
+use Bolt\Controller\Frontend\TemplateController;
+use Symfony\Component\ErrorHandler\ErrorRenderer\ErrorRendererInterface;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Controller\ErrorController as SymfonyErrorController;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Twig\Environment;
+use Twig\Error\LoaderError;
+
+class ErrorController extends SymfonyErrorController
+{
+    /** @var Config */
+    private $config;
+
+    /** @var DetailController */
+    private $detailController;
+
+    /** @var TemplateController */
+    private $templateController;
+
+    public function __construct(HttpKernelInterface $httpKernel, Config $config, DetailController $detailController, TemplateController $templateController, ErrorRendererInterface $errorRenderer)
+    {
+        $this->config = $config;
+        $this->detailController = $detailController;
+        $this->templateController = $templateController;
+
+        parent::__construct($httpKernel, $templateController, $errorRenderer);
+    }
+
+    /**
+     * Show an exception. Mainly used for custom 404 pages, otherwise falls back
+     * to Symfony's error handling
+     */
+    public function showAction(Environment $twig, \Throwable $exception): Response
+    {
+        if (method_exists($exception, 'getStatusCode')) {
+            $code = $exception->getStatusCode();
+        } else {
+            $code = Response::HTTP_INTERNAL_SERVER_ERROR;
+        }
+
+        if ($code === Response::HTTP_NOT_FOUND) {
+            $twig->addGlobal('exception', $exception);
+
+            // If Maintenance is on, show that, instead of the 404.
+            if ($this->isMaintenanceEnabled()) {
+                return $this->showMaintenance();
+            }
+
+            return $this->showNotFound();
+        }
+
+        if ($code === Response::HTTP_SERVICE_UNAVAILABLE) {
+            $twig->addGlobal('exception', $exception);
+
+            return $this->showMaintenance();
+        }
+
+        // If not a 404, we'll let Symfony handle it as usual.
+        return parent::__invoke($exception);
+    }
+
+    private function showNotFound(): Response
+    {
+        foreach ($this->config->get('general/notfound') as $item) {
+            $output = $this->attemptToRender($item);
+
+            if ($output instanceof Response) {
+                return $output;
+            }
+        }
+
+        return new Response('404: Not found (and there was no proper page configured to display)');
+    }
+
+    private function isMaintenanceEnabled()
+    {
+        return $this->config->get('general/maintenance_mode', false);
+    }
+
+    private function showMaintenance(): Response
+    {
+        foreach ($this->config->get('general/maintenance') as $item) {
+            $output = $this->attemptToRender($item);
+
+            if ($output instanceof Response) {
+                return $output;
+            }
+        }
+
+        return new Response('503: Maintenance mode (and there was no proper page configured to display)');
+    }
+
+    private function attemptToRender(string $item): ?Response
+    {
+        // First, see if it's a contenttype/slug pair:
+        [$contentType, $slug] = explode('/', $item . '/');
+
+        if (! empty($contentType) && ! empty($slug)) {
+            // We wrap it in a try/catch, because we wouldn't want to
+            // trigger a 404 within a 404 now, would we?
+            try {
+                return $this->detailController->record($slug, $contentType, false);
+            } catch (NotFoundHttpException $e) {
+                // Just continue to the next one.
+            }
+        }
+
+        // Then, let's see if it's a template we can render.
+        try {
+            return $this->templateController->template($item);
+        } catch (LoaderError $e) {
+            // Just continue to the next one.
+        }
+
+        return null;
+    }
+}
diff --git a/src/Controller/ExceptionController.php b/src/Controller/ExceptionController.php
@@ -16,6 +16,9 @@
 use Twig\Environment;
 use Twig\Error\LoaderError;
 
+/**
+ * @deprecated since Bolt 4.0, use Bolt\Controller\ErrorController instead
+ */
 class ExceptionController extends SymfonyExceptionController
 {
     /** @var Config */

diff --git a/src/Controller/Frontend/ListingController.php b/src/Controller/Frontend/ListingController.php
@@ -42,7 +42,7 @@ public function __construct(TemplateChooser $templateChooser, Query $query)
     public function listing(ContentRepository $contentRepository, Request $request, string $contentTypeSlug): Response
     {
         $contentType = ContentType::factory($contentTypeSlug, $this->config->get('contenttypes'));
-        $page = (int) $request->query->get('page', 1);
+        $page = (int) $request->query->get('page', '1');
         $amountPerPage = $contentType->get('listing_records');
         $order = $request->query->get('order', $contentType->get('order'));
 

diff --git a/src/Controller/Frontend/SearchController.php b/src/Controller/Frontend/SearchController.php
@@ -34,7 +34,7 @@ public function __construct(TemplateChooser $templateChooser, Sanitiser $sanitis
      */
     public function search(ContentRepository $contentRepository, Request $request): Response
     {
-        $page = (int) $request->query->get('page', 1);
+        $page = (int) $request->query->get('page', '1');
         $searchTerm = $request->get('searchTerm', $request->get('search', $request->get('q', '')));
         $searchTerm = $this->sanitiser->clean($searchTerm);
         $amountPerPage = (int) $this->config->get('general/listing_records');

diff --git a/src/Controller/Frontend/TaxonomyController.php b/src/Controller/Frontend/TaxonomyController.php
@@ -38,7 +38,7 @@ public function __construct(TemplateChooser $templateChooser)
      */
     public function listing(ContentRepository $contentRepository, Request $request, string $taxonomyslug, string $slug): Response
     {
-        $page = (int) $request->query->get('page', 1);
+        $page = (int) $request->query->get('page', '1');
         $amountPerPage = $this->config->get('general/listing_records');
 
         $taxonomy = $this->config->getTaxonomy($taxonomyslug);

diff --git a/src/Event/Subscriber/ZoneSubscriber.php b/src/Event/Subscriber/ZoneSubscriber.php
@@ -55,7 +55,14 @@ protected function determineZone(Request $request): string
             return RequestZone::ASYNC;
         }
 
-        $controller = explode('::', $request->attributes->get('_controller'));
+        $controller = $request->attributes->get('_controller');
+
+        // If this happens, we're usually in the middle of handling an Exception
+        if (! is_string($controller)) {
+            return RequestZone::NOWHERE;
+        }
+
+        $controller = explode('::', $controller);
 
         try {
             $reflection = new ReflectionClass($controller[0]);
@@ -68,7 +75,7 @@ protected function determineZone(Request $request): string
                 return RequestZone::ASYNC;
             }
         } catch (\ReflectionException $e) {
-            // Alas..
+            // Alas…
         }
 
         return RequestZone::NOWHERE;

diff --git a/templates/helpers/page_404.html.twig b/templates/helpers/page_404.html.twig
@@ -2,21 +2,21 @@
 <html lang="en">
 <head>
     <meta charset="utf-8">
-    <title>{{ exception.statusCode }} :: {{ exception.class|split('\\')|last }}</title>
+    <title>{{ exception.statusCode }} :: {{ exception.message }}</title>
     <link rel="stylesheet" href="https://unpkg.com/purecss@1.0.1/build/pure-min.css" integrity="sha384-oAOxQR6DkCoMliIh8yFnu25d7Eq/PHS21PClpwjOTeU2jRSq11vu66rf90/cZr47" crossorigin="anonymous">
 </head>
 
 <body id="home">
 
-<div style="padding: 2rem 4rem;">
+    <div style="padding: 2rem 4rem;">
 
-<h1>{{ exception.statusCode }} :: {{ exception.class|split('\\')|last }}</h1>
+        <h1>{{ exception.statusCode }} :: {{ exception.message }} </h1>
 
-<p>{{ exception.message }}</p>
+        <p>In <code>{{ exception.file|split('/')|slice(-2)|join('/') }}</code></p>
 
-    {# will only be shown if debug is on. #}
-    {{ dump(exception) }}
+        {# will only be shown if debug is on. #}
+        {{ dump(exception) }}
 
-</div>
+    </div>
 </body>
 </html>
diff --git a/templates/helpers/page_503.html.twig b/templates/helpers/page_503.html.twig
@@ -2,21 +2,21 @@
 <html lang="en">
 <head>
     <meta charset="utf-8">
-    <title>{{ exception.statusCode }} :: {{ exception.class|split('\\')|last }}</title>
+    <title>{{ exception.statusCode }} :: {{ exception.message }}</title>
     <link rel="stylesheet" href="https://unpkg.com/purecss@1.0.1/build/pure-min.css" integrity="sha384-oAOxQR6DkCoMliIh8yFnu25d7Eq/PHS21PClpwjOTeU2jRSq11vu66rf90/cZr47" crossorigin="anonymous">
 </head>
 
 <body id="home">
 
-<div style="padding: 2rem 4rem;">
+    <div style="padding: 2rem 4rem;">
 
-<h1>{{ exception.statusCode }} :: {{ exception.class|split('\\')|last }}</h1>
+        <h1>{{ exception.statusCode }} :: {{ exception.message }}</h1>
 
-<p>{{ exception.message }}</p>
+        <p>In <code>{{ exception.file|split('/')|slice(-2)|join('/') }}</code></p>
 
-    {# will only be shown if debug is on. #}
-    {{ dump(exception) }}
+        {# will only be shown if debug is on. #}
+        {{ dump(exception) }}
 
-</div>
+    </div>
 </body>
 </html>