diff --git a/CHANGELOG.md b/CHANGELOG.md index 9800cfb0..21154d40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 1.20.0 - 2019-11-02 #### Added @@ -14,6 +14,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this Generated short codes have 5 characters, and shlink makes sure they keep unique, while making it backwards-compatible. +* [#418](https://github.com/shlinkio/shlink/issues/418) and [#419](https://github.com/shlinkio/shlink/issues/419) Added support to redirect any 404 error to a custom URL. + + It was already possible to configure this but only for invalid short URLs. Shlink now also support configuring redirects for the base URL and any other kind of "not found" error. + + The three URLs can be different, and it is already possible to pass them to the docker image via configuration or env vars. + + The installer also asks for these two new configuration options. + +* [#497](https://github.com/shlinkio/shlink/issues/497) Officially added support for MariaDB. + #### Changed * [#458](https://github.com/shlinkio/shlink/issues/458) Updated coding styles to use [shlinkio/php-coding-standard](https://github.com/shlinkio/php-coding-standard) v2.0.0. diff --git a/composer.json b/composer.json index 54a3ea76..23ccb5c3 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,7 @@ "pugx/shortid-php": "^0.5", "shlinkio/shlink-common": "^2.2.1", "shlinkio/shlink-event-dispatcher": "^1.0", - "shlinkio/shlink-installer": "^2.1", + "shlinkio/shlink-installer": "^3.0", "shlinkio/shlink-ip-geolocation": "^1.1", "symfony/console": "^4.3", "symfony/filesystem": "^4.3", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index edc4a931..c610c11e 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -12,8 +12,6 @@ return [ Plugin\UrlShortenerConfigCustomizer::HOSTNAME, Plugin\UrlShortenerConfigCustomizer::CHARS, Plugin\UrlShortenerConfigCustomizer::VALIDATE_URL, - Plugin\UrlShortenerConfigCustomizer::ENABLE_NOT_FOUND_REDIRECTION, - Plugin\UrlShortenerConfigCustomizer::NOT_FOUND_REDIRECT_TO, ], Plugin\ApplicationConfigCustomizer::class => [ @@ -32,6 +30,12 @@ return [ Plugin\DatabaseConfigCustomizer::HOST, Plugin\DatabaseConfigCustomizer::PORT, ], + + Plugin\RedirectsConfigCustomizer::class => [ + Plugin\RedirectsConfigCustomizer::INVALID_SHORT_URL_REDIRECT_TO, + Plugin\RedirectsConfigCustomizer::REGULAR_404_REDIRECT_TO, + Plugin\RedirectsConfigCustomizer::BASE_URL_REDIRECT_TO, + ], ], 'installation_commands' => [ diff --git a/config/autoload/redirects.global.php b/config/autoload/redirects.global.php new file mode 100644 index 00000000..90137aa8 --- /dev/null +++ b/config/autoload/redirects.global.php @@ -0,0 +1,13 @@ + [ + 'invalid_short_url' => null, // Formerly url_shortener.not_found_short_url.redirect_to + 'regular_404' => null, + 'base_url' => null, + ], + +]; diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 3d3689ea..7c7e19de 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -12,10 +12,6 @@ return [ 'hostname' => env('SHORTENED_URL_HOSTNAME'), ], 'validate_url' => true, - 'not_found_short_url' => [ - 'enable_redirection' => false, - 'redirect_to' => null, - ], ], ]; diff --git a/config/config.php b/config/config.php index ed04d037..352a27ca 100644 --- a/config/config.php +++ b/config/config.php @@ -31,4 +31,5 @@ return (new ConfigAggregator\ConfigAggregator([ ], 'data/cache/app_config.php', [ Core\Config\SimplifiedConfigParser::class, Core\Config\BasePathPrefixer::class, + Core\Config\DeprecatedConfigParser::class, ]))->getMergedConfig(); diff --git a/docker/README.md b/docker/README.md index eca69d36..ebeee0ba 100644 --- a/docker/README.md +++ b/docker/README.md @@ -101,7 +101,9 @@ This is the complete list of supported env vars: * `DISABLE_TRACK_PARAM`: The name of a query param that can be used to visit short URLs avoiding the visit to be tracked. This feature won't be available if not value is provided. * `DELETE_SHORT_URL_THRESHOLD`: The amount of visits on short URLs which will not allow them to be deleted. Defaults to `15`. * `VALIDATE_URLS`: Boolean which tells if shlink should validate a status 20x (after following redirects) is returned when trying to shorten a URL. Defaults to `true`. -* `NOT_FOUND_REDIRECT_TO`: If a URL is provided here, when a user tries to access an invalid short URL, he/she will be redirected to this value. If this env var is not provided, the user will see a generic `404 - not found` page. +* `INVALID_SHORT_URL_REDIRECT_TO`: If a URL is provided here, when a user tries to access an invalid short URL, he/she will be redirected to this value. If this env var is not provided, the user will see a generic `404 - not found` page. +* `REGULAR_404_REDIRECT_TO`: If a URL is provided here, when a user tries to access a URL not matching any one supported by the router, he/she will be redirected to this value. If this env var is not provided, the user will see a generic `404 - not found` page. +* `BASE_URL_REDIRECT_TO`: If a URL is provided here, when a user tries to access Shlink's base URL, he/she will be redirected to this value. If this env var is not provided, the user will see a generic `404 - not found` page. * `BASE_PATH`: The base path from which you plan to serve shlink, in case you don't want to serve it from the root of the domain. Defaults to `''`. * `REDIS_SERVERS`: A comma-separated list of redis servers where Shlink locks are stored (locks are used to prevent some operations to be run more than once in parallel). @@ -111,6 +113,7 @@ This is the complete list of supported env vars: In the future, these redis servers could be used for other caching operations performed by shlink. +* `NOT_FOUND_REDIRECT_TO`: **Deprecated since v1.20 in favor of `INVALID_SHORT_URL_REDIRECT_TO`** If a URL is provided here, when a user tries to access an invalid short URL, he/she will be redirected to this value. If this env var is not provided, the user will see a generic `404 - not found` page. * `SHORTCODE_CHARS`: **Ignored when using Shlink 1.20 or newer**. A charset to use when building short codes. Only needed when using more than one shlink instance ([Multi instance considerations](#multi-instance-considerations)). An example using all env vars could look like this: @@ -130,7 +133,9 @@ docker run \ -e DISABLE_TRACK_PARAM="no-track" \ -e DELETE_SHORT_URL_THRESHOLD=30 \ -e VALIDATE_URLS=false \ - -e "NOT_FOUND_REDIRECT_TO=https://www.google.com" \ + -e "INVALID_SHORT_URL_REDIRECT_TO=https://my-landing-page.com" \ + -e "REGULAR_404_REDIRECT_TO=https://my-landing-page.com" \ + -e "BASE_URL_REDIRECT_TO=https://my-landing-page.com" \ -e "REDIS_SERVERS=tcp://172.20.0.1:6379,tcp://172.20.0.2:6379" \ -e "BASE_PATH=/my-campaign" \ shlinkio/shlink @@ -151,7 +156,9 @@ The whole configuration should have this format, but it can be split into multip "short_domain_schema": "https", "short_domain_host": "doma.in", "validate_url": false, - "not_found_redirect_to": "https://my-landing-page.com", + "invalid_short_url_redirect_to": "https://my-landing-page.com", + "regular_404_redirect_to": "https://my-landing-page.com", + "base_url_redirect_to": "https://my-landing-page.com", "redis_servers": [ "tcp://172.20.0.1:6379", "tcp://172.20.0.2:6379" @@ -163,11 +170,13 @@ The whole configuration should have this format, but it can be split into multip "password": "123abc", "host": "something.rds.amazonaws.com", "port": "3306" - } + }, + "not_found_redirect_to": "https://my-landing-page.com" } ``` > This is internally parsed to how shlink expects the config. If you are using a version previous to 1.17.0, this parser is not present and you need to provide a config structure like the one [documented previously](https://github.com/shlinkio/shlink-docker-image/tree/v1.16.3#provide-config-via-volumes). +> The `not_found_redirect_to` option has been deprecated when `regular_404_redirect_to` and `base_url_redirect_to` have been introduced. Use `invalid_short_url_redirect_to` instead (however, it will still work for backwards compatibility). Once created just run shlink with the volume: diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 5477e892..299a7c8e 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -77,7 +77,7 @@ $helper = new class { } $driverOptions = ! contains(['maria', 'mysql'], $driver) ? [] : [ - // PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8', + // 1002 -> PDO::MYSQL_ATTR_INIT_COMMAND 1002 => 'SET NAMES utf8', ]; return [ @@ -91,13 +91,12 @@ $helper = new class { ]; } - public function getNotFoundConfig(): array + public function getNotFoundRedirectsConfig(): array { - $notFoundRedirectTo = env('NOT_FOUND_REDIRECT_TO'); - return [ - 'enable_redirection' => $notFoundRedirectTo !== null, - 'redirect_to' => $notFoundRedirectTo, + 'invalid_short_url' => env('INVALID_SHORT_URL_REDIRECT_TO', env('NOT_FOUND_REDIRECT_TO')), + 'regular_404' => env('REGULAR_404_REDIRECT_TO'), + 'base_url' => env('BASE_URL_REDIRECT_TO'), ]; } }; @@ -126,9 +125,10 @@ return [ 'hostname' => env('SHORT_DOMAIN_HOST', ''), ], 'validate_url' => (bool) env('VALIDATE_URLS', true), - 'not_found_short_url' => $helper->getNotFoundConfig(), ], + 'not_found_redirects' => $helper->getNotFoundRedirectsConfig(), + 'logger' => [ 'handlers' => [ 'shlink_rotating_handler' => [ diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8ea3773c..a5d22179 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core; use Doctrine\Common\Cache\Cache; use Psr\EventDispatcher\EventDispatcherInterface; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGenerator; use Zend\Expressive\Router\RouterInterface; @@ -20,7 +21,7 @@ return [ Options\AppOptions::class => ConfigAbstractFactory::class, Options\DeleteShortUrlsOptions::class => ConfigAbstractFactory::class, - Options\NotFoundShortUrlOptions::class => ConfigAbstractFactory::class, + Options\NotFoundRedirectOptions::class => ConfigAbstractFactory::class, Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, Service\UrlShortener::class => ConfigAbstractFactory::class, @@ -40,11 +41,15 @@ return [ ], ConfigAbstractFactory::class => [ - NotFoundHandler::class => [TemplateRendererInterface::class], + NotFoundHandler::class => [ + TemplateRendererInterface::class, + NotFoundRedirectOptions::class, + 'config.router.base_path', + ], Options\AppOptions::class => ['config.app_options'], Options\DeleteShortUrlsOptions::class => ['config.delete_short_urls'], - Options\NotFoundShortUrlOptions::class => ['config.url_shortener.not_found_short_url'], + Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'], Options\UrlShortenerOptions::class => ['config.url_shortener'], Service\UrlShortener::class => ['httpClient', 'em', Options\UrlShortenerOptions::class], @@ -58,7 +63,6 @@ return [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, - Options\NotFoundShortUrlOptions::class, 'Logger_Shlink', ], Action\PixelAction::class => [ diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 64ae1b09..b38222df 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -11,7 +11,7 @@ return [ 'routes' => [ [ - 'name' => 'long-url-redirect', + 'name' => Action\RedirectAction::class, 'path' => '/{shortCode}', 'middleware' => [ IpAddress::class, @@ -20,7 +20,7 @@ return [ 'allowed_methods' => [RequestMethod::METHOD_GET], ], [ - 'name' => 'pixel-tracking', + 'name' => Action\PixelAction::class, 'path' => '/{shortCode}/track', 'middleware' => [ IpAddress::class, @@ -29,7 +29,7 @@ return [ 'allowed_methods' => [RequestMethod::METHOD_GET], ], [ - 'name' => 'short-url-qr-code', + 'name' => Action\QrCodeAction::class, 'path' => '/{shortCode}/qr-code[/{size:[0-9]+}]', 'middleware' => [ Middleware\QrCodeCacheMiddleware::class, diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index e732f6ef..4ac2a50c 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -11,7 +11,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\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; @@ -22,7 +21,6 @@ use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGeneratorInterface; class PreviewAction implements MiddlewareInterface { use ResponseUtilsTrait; - use ErrorResponseBuilderTrait; /** @var PreviewGeneratorInterface */ private $previewGenerator; @@ -60,7 +58,7 @@ class PreviewAction implements MiddlewareInterface return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException | EntityDoesNotExistException | PreviewGenerationException $e) { $this->logger->warning('An error occurred while generating preview image. {e}', ['e' => $e]); - return $this->buildErrorResponse($request, $handler); + return $handler->handle($request); } } } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index d03e4c4d..a1fdae5b 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -12,7 +12,6 @@ use Psr\Http\Server\RequestHandlerInterface; 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; @@ -21,8 +20,6 @@ use Zend\Expressive\Router\RouterInterface; class QrCodeAction implements MiddlewareInterface { - use ErrorResponseBuilderTrait; - private const DEFAULT_SIZE = 300; private const MIN_SIZE = 50; private const MAX_SIZE = 1000; @@ -65,10 +62,10 @@ class QrCodeAction implements MiddlewareInterface $this->urlShortener->shortCodeToUrl($shortCode, $domain); } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); - return $this->buildErrorResponse($request, $handler); + return $handler->handle($request); } - $path = $this->router->generateUri('long-url-redirect', ['shortCode' => $shortCode]); + $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $shortCode]); $size = $this->getSizeParam($request); $qrCode = new QrCode((string) $request->getUri()->withPath($path)->withQuery('')); diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 4f91b63f..31fdb1a3 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -7,31 +7,10 @@ namespace Shlinkio\Shlink\Core\Action; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; -use Shlinkio\Shlink\Core\Options; -use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; -use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Zend\Diactoros\Response\RedirectResponse; class RedirectAction extends AbstractTrackingAction { - use ErrorResponseBuilderTrait; - - /** @var Options\NotFoundShortUrlOptions */ - private $notFoundOptions; - - public function __construct( - UrlShortenerInterface $urlShortener, - VisitsTrackerInterface $visitTracker, - Options\AppOptions $appOptions, - Options\NotFoundShortUrlOptions $notFoundOptions, - ?LoggerInterface $logger = null - ) { - parent::__construct($urlShortener, $visitTracker, $appOptions, $logger); - $this->notFoundOptions = $notFoundOptions; - } - protected function createSuccessResp(string $longUrl): Response { // Return a redirect response to the long URL. @@ -39,14 +18,8 @@ class RedirectAction extends AbstractTrackingAction return new RedirectResponse($longUrl); } - protected function createErrorResp( - ServerRequestInterface $request, - RequestHandlerInterface $handler - ): Response { - if ($this->notFoundOptions->isRedirectionEnabled()) { - return new RedirectResponse($this->notFoundOptions->getRedirectTo()); - } - - return $this->buildErrorResponse($request, $handler); + protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response + { + return $handler->handle($request); } } diff --git a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php b/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php deleted file mode 100644 index b9046006..00000000 --- a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php +++ /dev/null @@ -1,21 +0,0 @@ -withAttribute(NotFoundHandler::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code'); - return $handler->handle($request); - } -} diff --git a/module/Core/src/Config/DeprecatedConfigParser.php b/module/Core/src/Config/DeprecatedConfigParser.php new file mode 100644 index 00000000..059c81a9 --- /dev/null +++ b/module/Core/src/Config/DeprecatedConfigParser.php @@ -0,0 +1,33 @@ + ['url_shortener', 'domain', 'schema'], 'short_domain_host' => ['url_shortener', 'domain', 'hostname'], 'validate_url' => ['url_shortener', 'validate_url'], - 'not_found_redirect_to' => ['url_shortener', 'not_found_short_url', 'redirect_to'], + 'not_found_redirect_to' => ['not_found_redirects', 'invalid_short_url'], // Deprecated + 'invalid_short_url_redirect_to' => ['not_found_redirects', 'invalid_short_url'], + 'regular_404_redirect_to' => ['not_found_redirects', 'regular_404'], + 'base_url_redirect_to' => ['not_found_redirects', 'base_path'], 'db_config' => ['entity_manager', 'connection'], 'delete_short_url_threshold' => ['delete_short_urls', 'visits_threshold'], 'redis_servers' => ['redis', 'servers'], 'base_path' => ['router', 'base_path'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ - 'not_found_redirect_to' => [ - 'path' => ['url_shortener', 'not_found_short_url', 'enable_redirection'], - 'value' => true, - ], 'delete_short_url_threshold' => [ 'path' => ['delete_short_urls', 'check_visits_threshold'], 'value' => true, @@ -43,9 +45,9 @@ class SimplifiedConfigParser public function __invoke(array $config): array { - $existingKeys = array_intersect_key($config, self::SIMPLIFIED_CONFIG_MAPPING); + $configForExistingKeys = $this->getConfigForKeysInMappingOrderedByMapping($config); - return reduce_left($existingKeys, function ($value, string $key, $c, PathCollection $collection) { + return reduce_left($configForExistingKeys, function ($value, string $key, $c, PathCollection $collection) { $path = self::SIMPLIFIED_CONFIG_MAPPING[$key]; if (contains(self::SIMPLIFIED_MERGEABLE_CONFIG, $key)) { $value = ArrayUtils::merge($collection->getValueInPath($path), $value); @@ -60,4 +62,20 @@ class SimplifiedConfigParser return $collection; }, new PathCollection($config))->toArray(); } + + private function getConfigForKeysInMappingOrderedByMapping(array $config): array + { + // Ignore any config which is not defined in the mapping + $configForExistingKeys = array_intersect_key($config, self::SIMPLIFIED_CONFIG_MAPPING); + + // Order the config by their key, based on the order it was defined in the mapping. + // This mainly allows deprecating keys and defining new ones that will replace the older and always take + // preference, while the old one keeps working for backwards compatibility if the new one is not provided. + $simplifiedConfigOrder = array_flip(array_keys(self::SIMPLIFIED_CONFIG_MAPPING)); + uksort($configForExistingKeys, function (string $a, string $b) use ($simplifiedConfigOrder): int { + return $simplifiedConfigOrder[$a] - $simplifiedConfigOrder[$b]; + }); + + return $configForExistingKeys; + } } diff --git a/module/Core/src/Options/NotFoundRedirectOptions.php b/module/Core/src/Options/NotFoundRedirectOptions.php new file mode 100644 index 00000000..0165f8fc --- /dev/null +++ b/module/Core/src/Options/NotFoundRedirectOptions.php @@ -0,0 +1,65 @@ +invalidShortUrl; + } + + public function hasInvalidShortUrlRedirect(): bool + { + return $this->invalidShortUrl !== null; + } + + protected function setInvalidShortUrl(?string $invalidShortUrl): self + { + $this->invalidShortUrl = $invalidShortUrl; + return $this; + } + + public function getRegular404Redirect(): ?string + { + return $this->regular404; + } + + public function hasRegular404Redirect(): bool + { + return $this->regular404 !== null; + } + + protected function setRegular404(?string $regular404): self + { + $this->regular404 = $regular404; + return $this; + } + + public function getBaseUrlRedirect(): ?string + { + return $this->baseUrl; + } + + public function hasBaseUrlRedirect(): bool + { + return $this->baseUrl !== null; + } + + protected function setBaseUrl(?string $baseUrl): self + { + $this->baseUrl = $baseUrl; + return $this; + } +} diff --git a/module/Core/src/Options/NotFoundShortUrlOptions.php b/module/Core/src/Options/NotFoundShortUrlOptions.php deleted file mode 100644 index 92c19e69..00000000 --- a/module/Core/src/Options/NotFoundShortUrlOptions.php +++ /dev/null @@ -1,37 +0,0 @@ -enableRedirection; - } - - protected function setEnableRedirection(bool $enableRedirection = true): self - { - $this->enableRedirection = $enableRedirection; - return $this; - } - - public function getRedirectTo(): string - { - return $this->redirectTo ?? ''; - } - - protected function setRedirectTo(?string $redirectTo): self - { - $this->redirectTo = $redirectTo; - return $this; - } -} diff --git a/module/Core/src/Response/NotFoundHandler.php b/module/Core/src/Response/NotFoundHandler.php index 5bf40661..6bf245c1 100644 --- a/module/Core/src/Response/NotFoundHandler.php +++ b/module/Core/src/Response/NotFoundHandler.php @@ -5,29 +5,42 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Response; use Fig\Http\Message\StatusCodeInterface; +use InvalidArgumentException; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Zend\Diactoros\Response; +use Zend\Expressive\Router\RouteResult; use Zend\Expressive\Template\TemplateRendererInterface; use function array_shift; use function explode; use function Functional\contains; +use function rtrim; class NotFoundHandler implements RequestHandlerInterface { - public const NOT_FOUND_TEMPLATE = 'notFoundTemplate'; + public const NOT_FOUND_TEMPLATE = 'ShlinkCore::error/404'; + public const INVALID_SHORT_CODE_TEMPLATE = 'ShlinkCore::invalid-short-code'; /** @var TemplateRendererInterface */ private $renderer; + /** @var NotFoundRedirectOptions */ + private $redirectOptions; /** @var string */ - private $defaultTemplate; + private $shlinkBasePath; - public function __construct(TemplateRendererInterface $renderer, string $defaultTemplate = 'ShlinkCore::error/404') - { + public function __construct( + TemplateRendererInterface $renderer, + NotFoundRedirectOptions $redirectOptions, + string $shlinkBasePath + ) { $this->renderer = $renderer; - $this->defaultTemplate = $defaultTemplate; + $this->redirectOptions = $redirectOptions; + $this->shlinkBasePath = $shlinkBasePath; } /** @@ -36,10 +49,17 @@ class NotFoundHandler implements RequestHandlerInterface * @param ServerRequestInterface $request * * @return ResponseInterface - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function handle(ServerRequestInterface $request): ResponseInterface { + /** @var RouteResult $routeResult */ + $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + $redirectResponse = $this->createRedirectResponse($routeResult, $request->getUri()); + if ($redirectResponse !== null) { + return $redirectResponse; + } + $accepts = explode(',', $request->getHeaderLine('Accept')); $accept = array_shift($accepts); $status = StatusCodeInterface::STATUS_NOT_FOUND; @@ -52,7 +72,30 @@ class NotFoundHandler implements RequestHandlerInterface ], $status); } - $notFoundTemplate = $request->getAttribute(self::NOT_FOUND_TEMPLATE, $this->defaultTemplate); - return new Response\HtmlResponse($this->renderer->render($notFoundTemplate), $status); + $template = $routeResult->isFailure() ? self::NOT_FOUND_TEMPLATE : self::INVALID_SHORT_CODE_TEMPLATE; + return new Response\HtmlResponse($this->renderer->render($template), $status); + } + + private function createRedirectResponse(RouteResult $routeResult, UriInterface $uri): ?ResponseInterface + { + $isBaseUrl = rtrim($uri->getPath(), '/') === $this->shlinkBasePath; + + if ($isBaseUrl && $this->redirectOptions->hasBaseUrlRedirect()) { + return new Response\RedirectResponse($this->redirectOptions->getBaseUrlRedirect()); + } + + if (!$isBaseUrl && $routeResult->isFailure() && $this->redirectOptions->hasRegular404Redirect()) { + return new Response\RedirectResponse($this->redirectOptions->getRegular404Redirect()); + } + + if ( + $routeResult->isSuccess() && + $routeResult->getMatchedRouteName() === RedirectAction::class && + $this->redirectOptions->hasInvalidShortUrlRedirect() + ) { + return new Response\RedirectResponse($this->redirectOptions->getInvalidShortUrlRedirect()); + } + + return null; } } diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index bd894f63..eec71dbd 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -25,20 +25,16 @@ class RedirectActionTest extends TestCase private $urlShortener; /** @var ObjectProphecy */ private $visitTracker; - /** @var Options\NotFoundShortUrlOptions */ - private $notFoundOptions; public function setUp(): void { $this->urlShortener = $this->prophesize(UrlShortener::class); $this->visitTracker = $this->prophesize(VisitsTracker::class); - $this->notFoundOptions = new Options\NotFoundShortUrlOptions(); $this->action = new RedirectAction( $this->urlShortener->reveal(), $this->visitTracker->reveal(), - new Options\AppOptions(['disableTrackParam' => 'foobar']), - $this->notFoundOptions + new Options\AppOptions(['disableTrackParam' => 'foobar']) ); } @@ -78,29 +74,6 @@ class RedirectActionTest extends TestCase $handle->shouldHaveBeenCalledOnce(); } - /** @test */ - public function redirectToCustomUrlIsReturnedIfConfiguredSoAndShortUrlIsNotFound(): void - { - $shortCode = 'abc123'; - $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow( - EntityDoesNotExistException::class - ); - - $handler = $this->prophesize(RequestHandlerInterface::class); - $handle = $handler->handle(Argument::any())->willReturn(new Response()); - - $this->notFoundOptions->enableRedirection = true; - $this->notFoundOptions->redirectTo = 'https://shlink.io'; - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $resp = $this->action->process($request, $handler->reveal()); - - $this->assertEquals(302, $resp->getStatusCode()); - $this->assertEquals('https://shlink.io', $resp->getHeaderLine('Location')); - $shortCodeToUrl->shouldHaveBeenCalledOnce(); - $handle->shouldNotHaveBeenCalled(); - } - /** @test */ public function visitIsNotTrackedIfDisableParamIsProvided(): void { diff --git a/module/Core/test/Config/DeprecatedConfigParserTest.php b/module/Core/test/Config/DeprecatedConfigParserTest.php new file mode 100644 index 00000000..be76ba80 --- /dev/null +++ b/module/Core/test/Config/DeprecatedConfigParserTest.php @@ -0,0 +1,95 @@ +postProcessor = new DeprecatedConfigParser(); + } + + /** @test */ + public function returnsConfigAsIsIfNewValueIsDefined(): void + { + $config = [ + 'not_found_redirects' => [ + 'invalid_short_url' => 'somewhere', + ], + ]; + + $result = ($this->postProcessor)($config); + + $this->assertEquals($config, $result); + } + + /** @test */ + public function doesNotProvideNewConfigIfOldOneIsDefinedButDisabled(): void + { + $config = [ + 'url_shortener' => [ + 'not_found_short_url' => [ + 'enable_redirection' => false, + 'redirect_to' => 'somewhere', + ], + ], + ]; + + $result = ($this->postProcessor)($config); + + $this->assertEquals($config, $result); + } + + /** @test */ + public function mapsOldConfigToNewOneWhenOldOneIsEnabled(): void + { + $config = [ + 'url_shortener' => [ + 'not_found_short_url' => [ + 'enable_redirection' => true, + 'redirect_to' => 'somewhere', + ], + ], + ]; + $expected = array_merge($config, [ + 'not_found_redirects' => [ + 'invalid_short_url' => 'somewhere', + ], + ]); + + $result = ($this->postProcessor)($config); + + $this->assertEquals($expected, $result); + } + + /** @test */ + public function definesNewConfigAsNullIfOldOneIsEnabledWithNoRedirectValue(): void + { + $config = [ + 'url_shortener' => [ + 'not_found_short_url' => [ + 'enable_redirection' => true, + ], + ], + ]; + $expected = array_merge($config, [ + 'not_found_redirects' => [ + 'invalid_short_url' => null, + ], + ]); + + $result = ($this->postProcessor)($config); + + $this->assertEquals($expected, $result); + } +} diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index c2b95fd5..f58cb5f5 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -75,10 +75,6 @@ class SimplifiedConfigParserTest extends TestCase 'hostname' => 'doma.in', ], 'validate_url' => false, - 'not_found_short_url' => [ - 'redirect_to' => 'foobar.com', - 'enable_redirection' => true, - ], ], 'delete_short_urls' => [ @@ -102,10 +98,38 @@ class SimplifiedConfigParserTest extends TestCase 'router' => [ 'base_path' => '/foo/bar', ], + + 'not_found_redirects' => [ + 'invalid_short_url' => 'foobar.com', + ], ]; $result = ($this->postProcessor)(array_merge($config, $simplified)); $this->assertEquals(array_merge($expected, $simplified), $result); } + + /** + * @test + * @dataProvider provideConfigWithDeprecates + */ + public function properlyMapsDeprecatedConfigs(array $config, string $expected): void + { + $result = ($this->postProcessor)($config); + $this->assertEquals($expected, $result['not_found_redirects']['invalid_short_url']); + } + + public function provideConfigWithDeprecates(): iterable + { + yield 'only deprecated config' => [['not_found_redirect_to' => 'old_value'], 'old_value']; + yield 'only new config' => [['invalid_short_url_redirect_to' => 'new_value'], 'new_value']; + yield 'both configs, new first' => [ + ['invalid_short_url_redirect_to' => 'new_value', 'not_found_redirect_to' => 'old_value'], + 'new_value', + ]; + yield 'both configs, deprecated first' => [ + ['not_found_redirect_to' => 'old_value', 'invalid_short_url_redirect_to' => 'new_value'], + 'new_value', + ]; + } } diff --git a/module/Core/test/Response/NotFoundHandlerTest.php b/module/Core/test/Response/NotFoundHandlerTest.php index 8ce2591d..8f1d6f51 100644 --- a/module/Core/test/Response/NotFoundHandlerTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -7,9 +7,17 @@ namespace ShlinkioTest\Shlink\Core\Response; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Server\MiddlewareInterface; +use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; +use Zend\Diactoros\ServerRequestFactory; +use Zend\Diactoros\Uri; +use Zend\Expressive\Router\Route; +use Zend\Expressive\Router\RouteResult; use Zend\Expressive\Template\TemplateRendererInterface; class NotFoundHandlerTest extends TestCase @@ -18,11 +26,15 @@ class NotFoundHandlerTest extends TestCase private $delegate; /** @var ObjectProphecy */ private $renderer; + /** @var NotFoundRedirectOptions */ + private $redirectOptions; public function setUp(): void { $this->renderer = $this->prophesize(TemplateRendererInterface::class); - $this->delegate = new NotFoundHandler($this->renderer->reveal()); + $this->redirectOptions = new NotFoundRedirectOptions(); + + $this->delegate = new NotFoundHandler($this->renderer->reveal(), $this->redirectOptions, ''); } /** @@ -47,4 +59,84 @@ class NotFoundHandlerTest extends TestCase yield 'application/x-json' => [Response\JsonResponse::class, 'application/x-json', 0]; yield 'text/html' => [Response\HtmlResponse::class, 'text/html', 1]; } + + /** + * @test + * @dataProvider provideRedirects + */ + public function expectedRedirectionIsReturnedDependingOnTheCase( + ServerRequestInterface $request, + string $expectedRedirectTo + ): void { + $this->redirectOptions->invalidShortUrl = 'invalidShortUrl'; + $this->redirectOptions->regular404 = 'regular404'; + $this->redirectOptions->baseUrl = 'baseUrl'; + + $resp = $this->delegate->handle($request); + + $this->assertInstanceOf(Response\RedirectResponse::class, $resp); + $this->assertEquals($expectedRedirectTo, $resp->getHeaderLine('Location')); + $this->renderer->render(Argument::cetera())->shouldNotHaveBeenCalled(); + } + + public function provideRedirects(): iterable + { + yield 'base URL with trailing slash' => [ + ServerRequestFactory::fromGlobals()->withUri(new Uri('/')), + 'baseUrl', + ]; + yield 'base URL without trailing slash' => [ + ServerRequestFactory::fromGlobals()->withUri(new Uri('')), + 'baseUrl', + ]; + yield 'regular 404' => [ + ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar')), + 'regular404', + ]; + yield 'invalid short URL' => [ + ServerRequestFactory::fromGlobals() + ->withAttribute( + RouteResult::class, + RouteResult::fromRoute( + new Route( + '', + $this->prophesize(MiddlewareInterface::class)->reveal(), + ['GET'], + RedirectAction::class + ) + ) + ) + ->withUri(new Uri('/abc123')), + 'invalidShortUrl', + ]; + } + + /** + * @test + * @dataProvider provideTemplates + */ + public function properErrorTemplateIsRendered(ServerRequestInterface $request, string $expectedTemplate): void + { + $request = $request->withHeader('Accept', 'text/html'); + $render = $this->renderer->render($expectedTemplate)->willReturn(''); + + $resp = $this->delegate->handle($request); + + $this->assertInstanceOf(Response\HtmlResponse::class, $resp); + $render->shouldHaveBeenCalledOnce(); + } + + public function provideTemplates(): iterable + { + $request = ServerRequestFactory::fromGlobals(); + + yield [$request, NotFoundHandler::NOT_FOUND_TEMPLATE]; + yield [ + $request->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal())) + ), + NotFoundHandler::INVALID_SHORT_CODE_TEMPLATE, + ]; + } }