Merge pull request #722 from shlinkio/develop

Release 2.1.3
This commit is contained in:
Alejandro Celaya 2020-04-09 12:50:46 +02:00 committed by GitHub
commit e775b0f12f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 118 additions and 152 deletions

View file

@ -4,6 +4,33 @@ 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).
## 2.1.3 - 2020-04-09
#### Added
* *Nothing*
#### Changed
* *Nothing*
#### Deprecated
* *Nothing*
#### Removed
* *Nothing*
#### Fixed
* [#712](https://github.com/shlinkio/shlink/issues/712) Fixed app set-up not clearing entities metadata cache.
* [#711](https://github.com/shlinkio/shlink/issues/711) Fixed `HEAD` requests returning a duplicated `Content-Length` header.
* [#716](https://github.com/shlinkio/shlink/issues/716) Fixed Twitter not properly displaying preview for final long URL.
* [#717](https://github.com/shlinkio/shlink/issues/717) Fixed DB connection expiring on task workers when using swoole.
* [#705](https://github.com/shlinkio/shlink/issues/705) Fixed how the short URL domain is inferred when generating QR codes, making sure the configured domain is respected even if the request is performed using a different one, and only when a custom domain is used, then that one is used instead.
## 2.1.2 - 2020-03-29
#### Added

View file

@ -52,7 +52,7 @@
"shlinkio/shlink-common": "^3.0",
"shlinkio/shlink-config": "^1.0",
"shlinkio/shlink-event-dispatcher": "^1.4",
"shlinkio/shlink-installer": "^4.3.1",
"shlinkio/shlink-installer": "^4.3.2",
"shlinkio/shlink-ip-geolocation": "^1.4",
"symfony/console": "^5.0",
"symfony/filesystem": "^5.0",
@ -65,7 +65,7 @@
"eaglewu/swoole-ide-helper": "dev-master",
"infection/infection": "^0.16.1",
"phpstan/phpstan": "^0.12.18",
"phpunit/phpunit": "^9.0.1",
"phpunit/phpunit": "~9.0.1",
"roave/security-advisories": "dev-master",
"shlinkio/php-coding-standard": "~2.1.0",
"shlinkio/shlink-test-utils": "^1.4",

View file

@ -5,8 +5,9 @@ declare(strict_types=1);
namespace Shlinkio\Shlink;
use Laminas\Stratigility\Middleware\ErrorHandler;
use Mezzio;
use Mezzio\Helper;
use Mezzio\ProblemDetails;
use Mezzio\Router;
use PhpMiddleware\RequestId\RequestIdMiddleware;
return [
@ -14,7 +15,7 @@ return [
'middleware_pipeline' => [
'error-handler' => [
'middleware' => [
Mezzio\Helper\ContentLengthMiddleware::class,
Helper\ContentLengthMiddleware::class,
ErrorHandler::class,
],
],
@ -35,14 +36,15 @@ return [
'routing' => [
'middleware' => [
Mezzio\Router\Middleware\RouteMiddleware::class,
Router\Middleware\RouteMiddleware::class,
Router\Middleware\ImplicitHeadMiddleware::class,
],
],
'rest' => [
'path' => '/rest',
'middleware' => [
Mezzio\Router\Middleware\ImplicitOptionsMiddleware::class,
Router\Middleware\ImplicitOptionsMiddleware::class,
Rest\Middleware\BodyParserMiddleware::class,
Rest\Middleware\AuthenticationMiddleware::class,
],
@ -50,7 +52,7 @@ return [
'dispatch' => [
'middleware' => [
Mezzio\Router\Middleware\DispatchMiddleware::class,
Router\Middleware\DispatchMiddleware::class,
],
],
@ -67,4 +69,5 @@ return [
],
],
],
];

View file

@ -12,6 +12,9 @@ php bin/cli db:migrate -n -q
echo "Generating proxies..."
php vendor/doctrine/orm/bin/doctrine.php orm:generate-proxies -n -q
echo "Clearing entities cache..."
php vendor/doctrine/orm/bin/doctrine.php orm:clear-cache:metadata -n -q
# When restarting the container, swoole might think it is already in execution
# This forces the app to be started every second until the exit code is 0
until php vendor/mezzio/mezzio-swoole/bin/mezzio-swoole start; do sleep 1 ; done

View file

@ -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'],
],

View file

@ -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],

View file

@ -4,7 +4,9 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Action;
use Fig\Http\Message\RequestMethodInterface;
use Laminas\Diactoros\Uri;
use Mezzio\Router\Middleware\ImplicitHeadMiddleware;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
@ -24,7 +26,7 @@ use function array_merge;
use function GuzzleHttp\Psr7\build_query;
use function GuzzleHttp\Psr7\parse_query;
abstract class AbstractTrackingAction implements MiddlewareInterface
abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface
{
private ShortUrlResolverInterface $urlResolver;
private VisitsTrackerInterface $visitTracker;
@ -50,14 +52,13 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
$disableTrackParam = $this->appOptions->getDisableTrackParam();
try {
$url = $this->urlResolver->resolveEnabledShortUrl($identifier);
$shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
// Track visit to this short code
if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) {
$this->visitTracker->track($url, Visitor::fromRequest($request));
if ($this->shouldTrackRequest($request, $query, $disableTrackParam)) {
$this->visitTracker->track($shortUrl, Visitor::fromRequest($request));
}
return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam));
return $this->createSuccessResp($this->buildUrlToRedirectTo($shortUrl, $query, $disableTrackParam));
} catch (ShortUrlNotFoundException $e) {
$this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]);
return $this->createErrorResp($request, $handler);
@ -76,6 +77,16 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
return (string) $uri->withQuery(build_query($mergedQuery));
}
private function shouldTrackRequest(ServerRequestInterface $request, array $query, ?string $disableTrackParam): bool
{
$forwardedMethod = $request->getAttribute(ImplicitHeadMiddleware::FORWARDED_HTTP_METHOD_ATTRIBUTE);
if ($forwardedMethod === self::METHOD_HEAD) {
return false;
}
return $disableTrackParam === null || ! array_key_exists($disableTrackParam, $query);
}
abstract protected function createSuccessResp(string $longUrl): ResponseInterface;
abstract protected function createErrorResp(

View file

@ -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);

View file

@ -9,6 +9,7 @@ use Psr\EventDispatcher\EventDispatcherInterface;
use Psr\Log\LoggerInterface;
use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface;
use Shlinkio\Shlink\Common\Doctrine\ReopeningEntityManager;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
@ -41,22 +42,35 @@ class LocateShortUrlVisit
public function __invoke(ShortUrlVisited $shortUrlVisited): void
{
// FIXME Temporarily handling DB connection reset here to fix https://github.com/shlinkio/shlink/issues/717
// Remove when https://github.com/shlinkio/shlink-event-dispatcher/issues/23 is implemented
if ($this->em instanceof ReopeningEntityManager) {
$this->em->open();
}
$visitId = $shortUrlVisited->visitId();
/** @var Visit|null $visit */
$visit = $this->em->find(Visit::class, $visitId);
if ($visit === null) {
$this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [
'visitId' => $visitId,
]);
return;
}
try {
/** @var Visit|null $visit */
$visit = $this->em->find(Visit::class, $visitId);
if ($visit === null) {
$this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [
'visitId' => $visitId,
]);
return;
}
if ($this->downloadOrUpdateGeoLiteDb($visitId)) {
$this->locateVisit($visitId, $shortUrlVisited->originalIpAddress(), $visit);
}
if ($this->downloadOrUpdateGeoLiteDb($visitId)) {
$this->locateVisit($visitId, $shortUrlVisited->originalIpAddress(), $visit);
}
$this->eventDispatcher->dispatch(new VisitLocated($visitId));
$this->eventDispatcher->dispatch(new VisitLocated($visitId));
} finally {
// FIXME Temporarily handling DB connection reset here to fix https://github.com/shlinkio/shlink/issues/717
// Remove when https://github.com/shlinkio/shlink-event-dispatcher/issues/23 is implemented
$this->em->getConnection()->close();
$this->em->clear();
}
}
private function downloadOrUpdateGeoLiteDb(string $visitId): bool

View file

@ -1,50 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Middleware;
use Doctrine\Common\Cache\Cache;
use Laminas\Diactoros\Response as DiactResp;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
class QrCodeCacheMiddleware implements MiddlewareInterface
{
private Cache $cache;
public function __construct(Cache $cache)
{
$this->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;
}
}

View file

@ -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 */

View file

@ -4,8 +4,10 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Action;
use Fig\Http\Message\RequestMethodInterface;
use Laminas\Diactoros\Response;
use Laminas\Diactoros\ServerRequest;
use Mezzio\Router\Middleware\ImplicitHeadMiddleware;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
@ -89,4 +91,23 @@ class RedirectActionTest extends TestCase
$handle->shouldHaveBeenCalledOnce();
}
/** @test */
public function trackingIsDisabledWhenRequestIsForwardedFromHead(): void
{
$shortCode = 'abc123';
$shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing');
$this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))->willReturn($shortUrl);
$track = $this->visitTracker->track(Argument::cetera())->will(function (): void {
});
$request = (new ServerRequest())->withAttribute('shortCode', $shortCode)
->withAttribute(
ImplicitHeadMiddleware::FORWARDED_HTTP_METHOD_ATTRIBUTE,
RequestMethodInterface::METHOD_HEAD,
);
$this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
$track->shouldNotHaveBeenCalled();
}
}

View file

@ -4,6 +4,7 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\EventDispatcher;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
@ -37,6 +38,10 @@ class LocateShortUrlVisitTest extends TestCase
{
$this->ipLocationResolver = $this->prophesize(IpLocationResolverInterface::class);
$this->em = $this->prophesize(EntityManagerInterface::class);
$conn = $this->prophesize(Connection::class);
$this->em->getConnection()->willReturn($conn->reveal());
$this->em->clear()->will(function (): void {
});
$this->logger = $this->prophesize(LoggerInterface::class);
$this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class);
$this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);

View file

@ -1,55 +0,0 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Middleware;
use Doctrine\Common\Cache\ArrayCache;
use Doctrine\Common\Cache\Cache;
use Laminas\Diactoros\Response;
use Laminas\Diactoros\ServerRequest;
use Laminas\Diactoros\Uri;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Core\Middleware\QrCodeCacheMiddleware;
class QrCodeCacheMiddlewareTest extends TestCase
{
private QrCodeCacheMiddleware $middleware;
private Cache $cache;
public function setUp(): void
{
$this->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);
}
}