From 551368c30d662b864f0551eb8a8c61519b3516ea Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 9 Apr 2020 12:31:03 +0200 Subject: [PATCH] Ensured QR code action respects configured domain --- module/Core/config/dependencies.config.php | 8 +-- module/Core/config/routes.config.php | 2 - module/Core/src/Action/QrCodeAction.php | 19 +++---- .../src/Middleware/QrCodeCacheMiddleware.php | 50 ----------------- module/Core/test/Action/QrCodeActionTest.php | 2 +- .../Middleware/QrCodeCacheMiddlewareTest.php | 55 ------------------- 6 files changed, 9 insertions(+), 127 deletions(-) delete mode 100644 module/Core/src/Middleware/QrCodeCacheMiddleware.php delete mode 100644 module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 13a74c36..a055e34b 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -4,9 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; -use Doctrine\Common\Cache\Cache; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; -use Mezzio\Router\RouterInterface; use Mezzio\Template\TemplateRendererInterface; use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Core\Domain\Resolver; @@ -39,8 +37,6 @@ return [ Action\PixelAction::class => ConfigAbstractFactory::class, Action\QrCodeAction::class => ConfigAbstractFactory::class, - Middleware\QrCodeCacheMiddleware::class => ConfigAbstractFactory::class, - Resolver\PersistenceDomainResolver::class => ConfigAbstractFactory::class, ], ], @@ -81,13 +77,11 @@ return [ 'Logger_Shlink', ], Action\QrCodeAction::class => [ - RouterInterface::class, Service\ShortUrl\ShortUrlResolver::class, + 'config.url_shortener.domain', 'Logger_Shlink', ], - Middleware\QrCodeCacheMiddleware::class => [Cache::class], - Resolver\PersistenceDomainResolver::class => ['em'], ], diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index d636ed23..82abef30 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -5,7 +5,6 @@ declare(strict_types=1); use Fig\Http\Message\RequestMethodInterface as RequestMethod; use RKA\Middleware\IpAddress; use Shlinkio\Shlink\Core\Action; -use Shlinkio\Shlink\Core\Middleware; return [ @@ -32,7 +31,6 @@ return [ 'name' => Action\QrCodeAction::class, 'path' => '/{shortCode}/qr-code[/{size:[0-9]+}]', 'middleware' => [ - Middleware\QrCodeCacheMiddleware::class, Action\QrCodeAction::class, ], 'allowed_methods' => [RequestMethod::METHOD_GET], diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 7a07f2a1..979e34fe 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; use Endroid\QrCode\QrCode; -use Mezzio\Router\RouterInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface; @@ -23,17 +22,17 @@ class QrCodeAction implements MiddlewareInterface private const MIN_SIZE = 50; private const MAX_SIZE = 1000; - private RouterInterface $router; private ShortUrlResolverInterface $urlResolver; + private array $domainConfig; private LoggerInterface $logger; public function __construct( - RouterInterface $router, ShortUrlResolverInterface $urlResolver, + array $domainConfig, ?LoggerInterface $logger = null ) { - $this->router = $router; $this->urlResolver = $urlResolver; + $this->domainConfig = $domainConfig; $this->logger = $logger ?: new NullLogger(); } @@ -42,23 +41,19 @@ class QrCodeAction implements MiddlewareInterface $identifier = ShortUrlIdentifier::fromRedirectRequest($request); try { - $this->urlResolver->resolveEnabledShortUrl($identifier); + $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier); } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); return $handler->handle($request); } - $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $identifier->shortCode()]); - $size = $this->getSizeParam($request); - - $qrCode = new QrCode((string) $request->getUri()->withPath($path)->withQuery('')); - $qrCode->setSize($size); + $qrCode = new QrCode($shortUrl->toString($this->domainConfig)); + $qrCode->setSize($this->getSizeParam($request)); $qrCode->setMargin(0); + return new QrCodeResponse($qrCode); } - /** - */ private function getSizeParam(Request $request): int { $size = (int) $request->getAttribute('size', self::DEFAULT_SIZE); diff --git a/module/Core/src/Middleware/QrCodeCacheMiddleware.php b/module/Core/src/Middleware/QrCodeCacheMiddleware.php deleted file mode 100644 index 79101ae5..00000000 --- a/module/Core/src/Middleware/QrCodeCacheMiddleware.php +++ /dev/null @@ -1,50 +0,0 @@ -cache = $cache; - } - - /** - * Process an incoming server request and return a response, optionally delegating - * to the next middleware component to create the response. - * - * - */ - public function process(Request $request, RequestHandlerInterface $handler): Response - { - $cacheKey = $request->getUri()->getPath(); - - // If this QR code is already cached, just return it - if ($this->cache->contains($cacheKey)) { - $qrData = $this->cache->fetch($cacheKey); - $response = new DiactResp(); - $response->getBody()->write($qrData['body']); - return $response->withHeader('Content-Type', $qrData['content-type']); - } - - // If not, call the next middleware and cache it - /** @var Response $resp */ - $resp = $handler->handle($request); - $this->cache->save($cacheKey, [ - 'body' => $resp->getBody()->__toString(), - 'content-type' => $resp->getHeaderLine('Content-Type'), - ]); - return $resp; - } -} diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 2a4d1a19..eb68f0e1 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -30,7 +30,7 @@ class QrCodeActionTest extends TestCase $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); - $this->action = new QrCodeAction($router->reveal(), $this->urlResolver->reveal()); + $this->action = new QrCodeAction($this->urlResolver->reveal(), ['domain' => 'doma.in']); } /** @test */ diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php deleted file mode 100644 index e2d73266..00000000 --- a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php +++ /dev/null @@ -1,55 +0,0 @@ -cache = new ArrayCache(); - $this->middleware = new QrCodeCacheMiddleware($this->cache); - } - - /** @test */ - public function noCachedPathFallsBackToNextMiddleware(): void - { - $delegate = $this->prophesize(RequestHandlerInterface::class); - $delegate->handle(Argument::any())->willReturn(new Response())->shouldBeCalledOnce(); - - $this->middleware->process((new ServerRequest())->withUri(new Uri('/foo/bar')), $delegate->reveal()); - - $this->assertTrue($this->cache->contains('/foo/bar')); - } - - /** @test */ - public function cachedPathReturnsCacheContent(): void - { - $isCalled = false; - $uri = (new Uri())->withPath('/foo'); - $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']); - $delegate = $this->prophesize(RequestHandlerInterface::class); - - $resp = $this->middleware->process((new ServerRequest())->withUri($uri), $delegate->reveal()); - - $this->assertFalse($isCalled); - $resp->getBody()->rewind(); - $this->assertEquals('the body', $resp->getBody()->getContents()); - $this->assertEquals('image/png', $resp->getHeaderLine('Content-Type')); - $delegate->handle(Argument::any())->shouldHaveBeenCalledTimes(0); - } -}