Ensured a generic template is used to render generic 404 errors, and a more specific one to render 'invalid short url' errors

module/Common/src/Util/ResponseUtilsTrait.php

@@ -3,13 +3,14 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Common\Util;
 
+use Psr\Http\Message\ResponseInterface;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\Stream;
 use Zend\Stdlib\ArrayUtils;
 
 trait ResponseUtilsTrait
 {
-    protected function generateDownloadFileResponse($filePath)
+    protected function generateDownloadFileResponse(string $filePath): ResponseInterface
     {
         return $this->generateBinaryResponse($filePath, [
             'Content-Disposition' => 'attachment; filename=' . basename($filePath),
@@ -21,12 +22,12 @@ trait ResponseUtilsTrait
         ]);
     }
 
-    protected function generateImageResponse($imagePath)
+    protected function generateImageResponse(string $imagePath): ResponseInterface
     {
         return $this->generateBinaryResponse($imagePath);
     }
 
-    protected function generateBinaryResponse($path, $extraHeaders = [])
+    protected function generateBinaryResponse(string $path, array $extraHeaders = []): ResponseInterface
     {
         $body = new Stream($path);
         return new Response($body, 200, ArrayUtils::merge([

module/Core/src/Action/PreviewAction.php

@@ -10,6 +10,7 @@ use Psr\Http\Message\ServerRequestInterface as Request;
 use Shlinkio\Shlink\Common\Exception\PreviewGenerationException;
 use Shlinkio\Shlink\Common\Service\PreviewGeneratorInterface;
 use Shlinkio\Shlink\Common\Util\ResponseUtilsTrait;
+use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
@@ -17,6 +18,7 @@ use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
 class PreviewAction implements MiddlewareInterface
 {
     use ResponseUtilsTrait;
+    use ErrorResponseBuilderTrait;
 
     /**
      * @var PreviewGeneratorInterface
@@ -51,11 +53,11 @@ class PreviewAction implements MiddlewareInterface
             $imagePath = $this->previewGenerator->generatePreview($url);
             return $this->generateImageResponse($imagePath);
         } catch (InvalidShortCodeException $e) {
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         } catch (EntityDoesNotExistException $e) {
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         } catch (PreviewGenerationException $e) {
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         }
     }
 }

module/Core/src/Action/QrCodeAction.php

@@ -11,6 +11,7 @@ use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Shlinkio\Shlink\Common\Response\QrCodeResponse;
+use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
@@ -18,6 +19,8 @@ use Zend\Expressive\Router\RouterInterface;
 
 class QrCodeAction implements MiddlewareInterface
 {
+    use ErrorResponseBuilderTrait;
+
     /**
      * @var RouterInterface
      */
@@ -55,19 +58,19 @@ class QrCodeAction implements MiddlewareInterface
         // Make sure the short URL exists for this short code
         $shortCode = $request->getAttribute('shortCode');
         try {
-            $shortUrl = $this->urlShortener->shortCodeToUrl($shortCode);
+            $this->urlShortener->shortCodeToUrl($shortCode);
         } catch (InvalidShortCodeException $e) {
             $this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e);
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         } catch (EntityDoesNotExistException $e) {
             $this->logger->warning('Tried to create a QR code with a not found short code' . PHP_EOL . $e);
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         }
 
         $path = $this->router->generateUri('long-url-redirect', ['shortCode' => $shortCode]);
         $size = $this->getSizeParam($request);
 
-        $qrCode = new QrCode($request->getUri()->withPath($path)->withQuery(''));
+        $qrCode = new QrCode((string) $request->getUri()->withPath($path)->withQuery(''));
         $qrCode->setSize($size)
                ->setPadding(0);
         return new QrCodeResponse($qrCode);

module/Core/src/Action/RedirectAction.php

@@ -9,6 +9,7 @@ use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
+use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
@@ -17,6 +18,8 @@ use Zend\Diactoros\Response\RedirectResponse;
 
 class RedirectAction implements MiddlewareInterface
 {
+    use ErrorResponseBuilderTrait;
+
     /**
      * @var UrlShortenerInterface
      */
@@ -69,9 +72,9 @@ class RedirectAction implements MiddlewareInterface
             // Use a temporary redirect to make sure browsers always hit the server for analytics purposes
             return new RedirectResponse($longUrl);
         } catch (InvalidShortCodeException $e) {
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         } catch (EntityDoesNotExistException $e) {
-            return $delegate->process($request);
+            return $this->buildErrorResponse($request, $delegate);
         }
     }
 }

module/Core/src/Action/Util/ErrorResponseBuilderTrait.php

@@ -0,0 +1,18 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Action\Util;
+
+use Interop\Http\ServerMiddleware\DelegateInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Shlinkio\Shlink\Core\Response\NotFoundDelegate;
+
+trait ErrorResponseBuilderTrait
+{
+    private function buildErrorResponse(ServerRequestInterface $request, DelegateInterface $delegate): ResponseInterface
+    {
+        $request = $request->withAttribute(NotFoundDelegate::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code');
+        return $delegate->process($request);
+    }
+}

module/Core/src/Response/NotFoundDelegate.php

@@ -12,6 +12,8 @@ use Zend\Expressive\Template\TemplateRendererInterface;
 
 class NotFoundDelegate implements DelegateInterface
 {
+    const NOT_FOUND_TEMPLATE = 'notFoundTemplate';
+
     /**
      * @var TemplateRendererInterface
      */
@@ -19,12 +21,12 @@ class NotFoundDelegate implements DelegateInterface
     /**
      * @var string
      */
-    private $template;
+    private $defaultTemplate;
 
-    public function __construct(TemplateRendererInterface $renderer, string $template = 'ShlinkCore::error/404')
+    public function __construct(TemplateRendererInterface $renderer, string $defaultTemplate = 'ShlinkCore::error/404')
     {
         $this->renderer = $renderer;
-        $this->template = $template;
+        $this->defaultTemplate = $defaultTemplate;
     }
 
     /**
@@ -49,6 +51,7 @@ class NotFoundDelegate implements DelegateInterface
             ], $status);
         }
 
-        return new Response\HtmlResponse($this->renderer->render($this->template, ['request' => $request]), $status);
+        $notFoundTemplate = $request->getAttribute(self::NOT_FOUND_TEMPLATE, $this->defaultTemplate);
+        return new Response\HtmlResponse($this->renderer->render($notFoundTemplate, ['request' => $request]), $status);
     }
 }

module/Core/templates/error/404.phtml

@@ -12,8 +12,8 @@
 <?php $this->end() ?>
 
 <?php $this->start('main') ?>
-    <h1><?= $this->translate('Oops!') ?></h1>
+    <h1>404</h1>
     <hr>
-    <p><?= $this->translate('This short URL doesn\'t seem to be valid.') ?></p>
-    <p><?= $this->translate('Make sure you included all the characters, with no extra punctuation.') ?></p>
+    <h3><?= $this->translate('Page not found.') ?></h3>
+    <p><?= $this->translate('The page you requested could not be found.') ?></p>
 <?php $this->end() ?>

module/Core/templates/invalid-short-code.phtml

@@ -0,0 +1,19 @@
+<?php $this->layout('ShlinkCore::layout/default') ?>
+
+<?php $this->start('title') ?>
+    <?= $this->translate('URL Not Found') ?>
+<?php $this->end() ?>
+
+<?php $this->start('stylesheets') ?>
+    <style>
+        p {margin-bottom: 20px;}
+        body {text-align: center;}
+    </style>
+<?php $this->end() ?>
+
+<?php $this->start('main') ?>
+    <h1><?= $this->translate('Oops!') ?></h1>
+    <hr>
+    <p><?= $this->translate('This short URL doesn\'t seem to be valid.') ?></p>
+    <p><?= $this->translate('Make sure you included all the characters, with no extra punctuation.') ?></p>
+<?php $this->end() ?>

module/Core/test/Action/PreviewActionTest.php

@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Action;
 use Interop\Http\ServerMiddleware\DelegateInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
+use Prophecy\Prophecy\MethodProphecy;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Common\Service\PreviewGenerator;
 use Shlinkio\Shlink\Core\Action\PreviewAction;
@@ -13,6 +14,7 @@ use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
 use ShlinkioTest\Shlink\Common\Util\TestUtils;
+use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequestFactory;
 
 class PreviewActionTest extends TestCase
@@ -46,7 +48,8 @@ class PreviewActionTest extends TestCase
         $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
                                                        ->shouldBeCalledTimes(1);
         $delegate = $this->prophesize(DelegateInterface::class);
-        $delegate->process(Argument::cetera())->shouldBeCalledTimes(1);
+        $delegate->process(Argument::cetera())->shouldBeCalledTimes(1)
+                                              ->willReturn(new Response());
 
         $this->action->process(
             ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode),
@@ -83,12 +86,14 @@ class PreviewActionTest extends TestCase
         $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class)
                                                        ->shouldBeCalledTimes(1);
         $delegate = $this->prophesize(DelegateInterface::class);
+        /** @var MethodProphecy $process */
+        $process = $delegate->process(Argument::any())->willReturn(new Response());
 
         $this->action->process(
             ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode),
             $delegate->reveal()
         );
 
-        $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1);
+        $process->shouldHaveBeenCalledTimes(1);
     }
 }

module/Core/test/Action/QrCodeActionTest.php

@@ -6,12 +6,14 @@ namespace ShlinkioTest\Shlink\Core\Action;
 use Interop\Http\ServerMiddleware\DelegateInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
+use Prophecy\Prophecy\MethodProphecy;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Common\Response\QrCodeResponse;
 use Shlinkio\Shlink\Core\Action\QrCodeAction;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
 use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
 use Shlinkio\Shlink\Core\Service\UrlShortener;
+use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequestFactory;
 use Zend\Expressive\Router\RouterInterface;
 
@@ -45,13 +47,14 @@ class QrCodeActionTest extends TestCase
         $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
                                                        ->shouldBeCalledTimes(1);
         $delegate = $this->prophesize(DelegateInterface::class);
+        $process = $delegate->process(Argument::any())->willReturn(new Response());
 
         $this->action->process(
             ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode),
             $delegate->reveal()
         );
 
-        $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1);
+        $process->shouldHaveBeenCalledTimes(1);
     }
 
     /**
@@ -63,13 +66,15 @@ class QrCodeActionTest extends TestCase
         $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class)
                                                        ->shouldBeCalledTimes(1);
         $delegate = $this->prophesize(DelegateInterface::class);
+        /** @var MethodProphecy $process */
+        $process = $delegate->process(Argument::any())->willReturn(new Response());
 
         $this->action->process(
             ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode),
             $delegate->reveal()
         );
 
-        $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1);
+        $process->shouldHaveBeenCalledTimes(1);
     }
 
     /**

module/Core/test/Action/RedirectActionTest.php

@@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Action;
 use Interop\Http\ServerMiddleware\DelegateInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
+use Prophecy\Prophecy\MethodProphecy;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Action\RedirectAction;
 use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
@@ -62,10 +63,12 @@ class RedirectActionTest extends TestCase
         $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
                                                        ->shouldBeCalledTimes(1);
         $delegate = $this->prophesize(DelegateInterface::class);
+        /** @var MethodProphecy $process */
+        $process = $delegate->process(Argument::any())->willReturn(new Response());
 
         $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
         $this->action->process($request, $delegate->reveal());
 
-        $delegate->process($request)->shouldHaveBeenCalledTimes(1);
+        $process->shouldHaveBeenCalledTimes(1);
     }
 }