diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d253e85..7e1542ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The new `forwardQuery=true|false` param can be provided during short URL creation or edition, via REST API or CLI command, allowing to override the default behavior which makes the query string to always be forwarded. +* [#1105](https://github.com/shlinkio/shlink/issues/1105) Added support to define placeholders on not-found redirects, so that the redirected URL receives the originally visited path and/or domain. + + Currently, `{DOMAIN}` and `{ORIGINAL_PATH}` placeholders are supported, and they can be used both in the redirected URL's path or query. + + When they are used in the query, the values are URL encoded. + ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. * [#1157](https://github.com/shlinkio/shlink/issues/1157) All routes now support CORS, not only rest ones. diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 66d854c3..7c3d7468 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -127,7 +127,7 @@ return [ Util\DoctrineBatchHelper::class => ['em'], Util\RedirectResponseHelper::class => [Options\UrlShortenerOptions::class], - Config\NotFoundRedirectResolver::class => [Util\RedirectResponseHelper::class], + Config\NotFoundRedirectResolver::class => [Util\RedirectResponseHelper::class, 'Logger_Shlink'], Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index a6a70c31..531254f7 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -4,31 +4,78 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; +use League\Uri\Exceptions\SyntaxError; +use League\Uri\Uri; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; +use function Functional\compose; +use function str_replace; + class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface { - public function __construct(private RedirectResponseHelperInterface $redirectResponseHelper) - { + private const DOMAIN_PLACEHOLDER = '{DOMAIN}'; + private const ORIGINAL_PATH_PLACEHOLDER = '{ORIGINAL_PATH}'; + + public function __construct( + private RedirectResponseHelperInterface $redirectResponseHelper, + private LoggerInterface $logger, + ) { } public function resolveRedirectResponse( NotFoundType $notFoundType, NotFoundRedirectConfigInterface $config, + UriInterface $currentUri, ): ?ResponseInterface { - return match (true) { - $notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectResponseHelper->buildRedirectResponse($config->baseUrlRedirect()), - $notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectResponseHelper->buildRedirectResponse($config->regular404Redirect()), + $urlToRedirectTo = match (true) { + $notFoundType->isBaseUrl() && $config->hasBaseUrlRedirect() => $config->baseUrlRedirect(), + $notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => $config->regular404Redirect(), $notFoundType->isInvalidShortUrl() && $config->hasInvalidShortUrlRedirect() => - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectResponseHelper->buildRedirectResponse($config->invalidShortUrlRedirect()), + $config->invalidShortUrlRedirect(), default => null, }; + + if ($urlToRedirectTo === null) { + return null; + } + + return $this->redirectResponseHelper->buildRedirectResponse( + $this->resolvePlaceholders($currentUri, $urlToRedirectTo), + ); + } + + private function resolvePlaceholders(UriInterface $currentUri, string $redirectUrl): string + { + $domain = $currentUri->getAuthority(); + $path = $currentUri->getPath(); + + try { + $redirectUri = Uri::createFromString($redirectUrl); + } catch (SyntaxError $e) { + $this->logger->warning('It was not possible to parse "{url}" as a valid URL: {e}', [ + 'e' => $e, + 'url' => $redirectUrl, + ]); + return $redirectUrl; + } + + $replacePlaceholderForPattern = static fn (string $pattern, string $replace, callable $modifier) => + static fn (?string $value) => + $value === null ? null : str_replace($modifier($pattern), $modifier($replace), $value); + $replacePlaceholders = static fn (callable $modifier) => compose( + $replacePlaceholderForPattern(self::DOMAIN_PLACEHOLDER, $domain, $modifier), + $replacePlaceholderForPattern(self::ORIGINAL_PATH_PLACEHOLDER, $path, $modifier), + ); + $replacePlaceholdersInPath = $replacePlaceholders('\Functional\id'); + $replacePlaceholdersInQuery = $replacePlaceholders('\urlencode'); + + return $redirectUri + ->withPath($replacePlaceholdersInPath($redirectUri->getPath())) + ->withQuery($replacePlaceholdersInQuery($redirectUri->getQuery())) + ->__toString(); } } diff --git a/module/Core/src/Config/NotFoundRedirectResolverInterface.php b/module/Core/src/Config/NotFoundRedirectResolverInterface.php index ab010d2e..6cbdf702 100644 --- a/module/Core/src/Config/NotFoundRedirectResolverInterface.php +++ b/module/Core/src/Config/NotFoundRedirectResolverInterface.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; interface NotFoundRedirectResolverInterface @@ -12,5 +13,6 @@ interface NotFoundRedirectResolverInterface public function resolveRedirectResponse( NotFoundType $notFoundType, NotFoundRedirectConfigInterface $config, + UriInterface $currentUri, ): ?ResponseInterface; } diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index 84918876..4138a72e 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\ErrorHandler; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolverInterface; @@ -26,19 +27,25 @@ class NotFoundRedirectHandler implements MiddlewareInterface { /** @var NotFoundType $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); - $authority = $request->getUri()->getAuthority(); - $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($authority, $notFoundType); + $currentUri = $request->getUri(); + $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($currentUri, $notFoundType); return $domainSpecificRedirect // If we did not find domain-specific redirects for current domain, we try to fall back to default redirects - ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions) + ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions, $currentUri) // Ultimately, we just call next handler if no domain-specific redirects or default redirects were found ?? $handler->handle($request); } - private function resolveDomainSpecificRedirect(string $authority, NotFoundType $notFoundType): ?ResponseInterface - { - $domain = $this->domainService->findByAuthority($authority); - return $domain === null ? null : $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain); + private function resolveDomainSpecificRedirect( + UriInterface $currentUri, + NotFoundType $notFoundType, + ): ?ResponseInterface { + $domain = $this->domainService->findByAuthority($currentUri->getAuthority()); + if ($domain === null) { + return null; + } + + return $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain, $currentUri); } } diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php index fe482a41..0dc25768 100644 --- a/module/Core/test/Config/NotFoundRedirectResolverTest.php +++ b/module/Core/test/Config/NotFoundRedirectResolverTest.php @@ -14,9 +14,10 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Action\RedirectAction; -use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolver; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; @@ -28,18 +29,11 @@ class NotFoundRedirectResolverTest extends TestCase private NotFoundRedirectResolver $resolver; private ObjectProphecy $helper; - private NotFoundRedirectConfigInterface $config; protected function setUp(): void { $this->helper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->resolver = new NotFoundRedirectResolver($this->helper->reveal()); - - $this->config = new NotFoundRedirectOptions([ - 'invalidShortUrl' => 'invalidShortUrl', - 'regular404' => 'regular404', - 'baseUrl' => 'baseUrl', - ]); + $this->resolver = new NotFoundRedirectResolver($this->helper->reveal(), new NullLogger()); } /** @@ -47,13 +41,15 @@ class NotFoundRedirectResolverTest extends TestCase * @dataProvider provideRedirects */ public function expectedRedirectionIsReturnedDependingOnTheCase( + UriInterface $uri, NotFoundType $notFoundType, + NotFoundRedirectOptions $redirectConfig, string $expectedRedirectTo, ): void { $expectedResp = new Response(); $buildResp = $this->helper->buildRedirectResponse($expectedRedirectTo)->willReturn($expectedResp); - $resp = $this->resolver->resolveRedirectResponse($notFoundType, $this->config); + $resp = $this->resolver->resolveRedirectResponse($notFoundType, $redirectConfig, $uri); self::assertSame($expectedResp, $resp); $buildResp->shouldHaveBeenCalledOnce(); @@ -62,21 +58,61 @@ class NotFoundRedirectResolverTest extends TestCase public function provideRedirects(): iterable { yield 'base URL with trailing slash' => [ - $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/'))), + $uri = new Uri('/'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'baseUrl']), 'baseUrl', ]; + yield 'base URL with domain placeholder' => [ + $uri = new Uri('https://doma.in'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'https://redirect-here.com/{DOMAIN}']), + 'https://redirect-here.com/doma.in', + ]; + yield 'base URL with domain placeholder in query' => [ + $uri = new Uri('https://doma.in'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'https://redirect-here.com/?domain={DOMAIN}']), + 'https://redirect-here.com/?domain=doma.in', + ]; yield 'base URL without trailing slash' => [ - $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri(''))), + $uri = new Uri(''), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['baseUrl' => 'baseUrl']), 'baseUrl', ]; yield 'regular 404' => [ - $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar'))), + $uri = new Uri('/foo/bar'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['regular404' => 'regular404']), 'regular404', ]; + yield 'regular 404 with path placeholder in query' => [ + $uri = new Uri('/foo/bar'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(['regular404' => 'https://redirect-here.com/?path={ORIGINAL_PATH}']), + 'https://redirect-here.com/?path=%2Ffoo%2Fbar', + ]; + yield 'regular 404 with multiple placeholders' => [ + $uri = new Uri('https://doma.in/foo/bar'), + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions([ + 'regular404' => 'https://redirect-here.com/{ORIGINAL_PATH}/{DOMAIN}/?d={DOMAIN}&p={ORIGINAL_PATH}', + ]), + 'https://redirect-here.com//foo/bar/doma.in/?d=doma.in&p=%2Ffoo%2Fbar', // TODO Fix duplicated slash + ]; yield 'invalid short URL' => [ + new Uri('/foo'), $this->notFoundType($this->requestForRoute(RedirectAction::class)), + new NotFoundRedirectOptions(['invalidShortUrl' => 'invalidShortUrl']), 'invalidShortUrl', ]; + yield 'invalid short URL with path placeholder' => [ + new Uri('/foo'), + $this->notFoundType($this->requestForRoute(RedirectAction::class)), + new NotFoundRedirectOptions(['invalidShortUrl' => 'https://redirect-here.com/{ORIGINAL_PATH}']), + 'https://redirect-here.com//foo', // TODO Fix duplicated slash + ]; } /** @test */ @@ -84,7 +120,7 @@ class NotFoundRedirectResolverTest extends TestCase { $notFoundType = $this->notFoundType($this->requestForRoute('foo')); - $result = $this->resolver->resolveRedirectResponse($notFoundType, $this->config); + $result = $this->resolver->resolveRedirectResponse($notFoundType, new NotFoundRedirectOptions(), new Uri()); self::assertNull($result); $this->helper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index e508a87b..70063764 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -11,6 +11,7 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\UriInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolverInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; @@ -75,6 +76,7 @@ class NotFoundRedirectHandlerTest extends TestCase $resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), Argument::type(NotFoundRedirectOptions::class), + Argument::type(UriInterface::class), )->willReturn(null)->shouldBeCalledOnce(); }]; yield 'non-redirecting domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { @@ -84,10 +86,13 @@ class NotFoundRedirectHandlerTest extends TestCase $resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), Argument::type(NotFoundRedirectOptions::class), + Argument::type(UriInterface::class), + )->willReturn(null)->shouldBeCalledOnce(); + $resolver->resolveRedirectResponse( + Argument::type(NotFoundType::class), + Argument::type(Domain::class), + Argument::type(UriInterface::class), )->willReturn(null)->shouldBeCalledOnce(); - $resolver->resolveRedirectResponse(Argument::type(NotFoundType::class), Argument::type(Domain::class)) - ->willReturn(null) - ->shouldBeCalledOnce(); }]; } @@ -100,6 +105,7 @@ class NotFoundRedirectHandlerTest extends TestCase $resolveRedirect = $this->resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), $this->redirectOptions, + Argument::type(UriInterface::class), )->willReturn($expectedResp); $result = $this->middleware->process($this->req, $this->next->reveal()); @@ -120,6 +126,7 @@ class NotFoundRedirectHandlerTest extends TestCase $resolveRedirect = $this->resolver->resolveRedirectResponse( Argument::type(NotFoundType::class), $domain, + Argument::type(UriInterface::class), )->willReturn($expectedResp); $result = $this->middleware->process($this->req, $this->next->reveal());