diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index df45336c..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; @@ -40,7 +41,11 @@ 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'], @@ -58,7 +63,6 @@ return [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, - Options\NotFoundRedirectOptions::class, 'Logger_Shlink', ], Action\PixelAction::class => [ diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index a7950fe6..0ae94235 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -7,11 +7,8 @@ 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 @@ -21,17 +18,6 @@ class RedirectAction extends AbstractTrackingAction /** @var Options\NotFoundRedirectOptions */ private $redirectOptions; - public function __construct( - UrlShortenerInterface $urlShortener, - VisitsTrackerInterface $visitTracker, - Options\AppOptions $appOptions, - Options\NotFoundRedirectOptions $redirectOptions, - ?LoggerInterface $logger = null - ) { - parent::__construct($urlShortener, $visitTracker, $appOptions, $logger); - $this->redirectOptions = $redirectOptions; - } - protected function createSuccessResp(string $longUrl): Response { // Return a redirect response to the long URL. @@ -39,14 +25,8 @@ class RedirectAction extends AbstractTrackingAction return new RedirectResponse($longUrl); } - protected function createErrorResp( - ServerRequestInterface $request, - RequestHandlerInterface $handler - ): Response { - if ($this->redirectOptions->hasInvalidShortUrlRedirect()) { - return new RedirectResponse($this->redirectOptions->getInvalidShortUrlRedirect()); - } - + protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response + { return $this->buildErrorResponse($request, $handler); } } diff --git a/module/Core/src/Response/NotFoundHandler.php b/module/Core/src/Response/NotFoundHandler.php index 5bf40661..d75d956f 100644 --- a/module/Core/src/Response/NotFoundHandler.php +++ b/module/Core/src/Response/NotFoundHandler.php @@ -5,15 +5,19 @@ 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\Server\RequestHandlerInterface; +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 { @@ -23,11 +27,21 @@ class NotFoundHandler implements RequestHandlerInterface private $renderer; /** @var string */ private $defaultTemplate; + /** @var NotFoundRedirectOptions */ + private $redirectOptions; + /** @var string */ + private $shlinkBasePath; - public function __construct(TemplateRendererInterface $renderer, string $defaultTemplate = 'ShlinkCore::error/404') - { + public function __construct( + TemplateRendererInterface $renderer, + NotFoundRedirectOptions $redirectOptions, + string $shlinkBasePath, + string $defaultTemplate = 'ShlinkCore::error/404' + ) { $this->renderer = $renderer; $this->defaultTemplate = $defaultTemplate; + $this->redirectOptions = $redirectOptions; + $this->shlinkBasePath = $shlinkBasePath; } /** @@ -36,10 +50,15 @@ class NotFoundHandler implements RequestHandlerInterface * @param ServerRequestInterface $request * * @return ResponseInterface - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function handle(ServerRequestInterface $request): ResponseInterface { + $redirectResponse = $this->createRedirectResponse($request); + if ($redirectResponse !== null) { + return $redirectResponse; + } + $accepts = explode(',', $request->getHeaderLine('Accept')); $accept = array_shift($accepts); $status = StatusCodeInterface::STATUS_NOT_FOUND; @@ -55,4 +74,29 @@ class NotFoundHandler implements RequestHandlerInterface $notFoundTemplate = $request->getAttribute(self::NOT_FOUND_TEMPLATE, $this->defaultTemplate); return new Response\HtmlResponse($this->renderer->render($notFoundTemplate), $status); } + + private function createRedirectResponse(ServerRequestInterface $request): ?ResponseInterface + { + /** @var RouteResult $routeResult */ + $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + $isBaseUrl = rtrim($request->getUri()->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() === 'long-url-redirect' && + $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 af28b1aa..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\NotFoundRedirectOptions */ - private $redirectOptions; public function setUp(): void { $this->urlShortener = $this->prophesize(UrlShortener::class); $this->visitTracker = $this->prophesize(VisitsTracker::class); - $this->redirectOptions = new Options\NotFoundRedirectOptions(); $this->action = new RedirectAction( $this->urlShortener->reveal(), $this->visitTracker->reveal(), - new Options\AppOptions(['disableTrackParam' => 'foobar']), - $this->redirectOptions + new Options\AppOptions(['disableTrackParam' => 'foobar']) ); } @@ -78,28 +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->redirectOptions->invalidShortUrl = '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/Response/NotFoundHandlerTest.php b/module/Core/test/Response/NotFoundHandlerTest.php index 8ce2591d..bd18a32a 100644 --- a/module/Core/test/Response/NotFoundHandlerTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -7,9 +7,16 @@ 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\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 +25,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 +58,55 @@ 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'], + 'long-url-redirect' + ) + ) + ) + ->withUri(new Uri('/abc123')), + 'invalidShortUrl', + ]; + } }