diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 436810e6..4d883794 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -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( diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 8d28f21c..f4de05c5 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -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(); + } }