Ensured redirect requests are not tracked when request is performed using method HEAD

This commit is contained in:
Alejandro Celaya 2020-04-08 23:51:57 +02:00
parent ec8cbf82e5
commit 1980d35691
2 changed files with 38 additions and 6 deletions

View file

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

View file

@ -4,8 +4,10 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Action; namespace ShlinkioTest\Shlink\Core\Action;
use Fig\Http\Message\RequestMethodInterface;
use Laminas\Diactoros\Response; use Laminas\Diactoros\Response;
use Laminas\Diactoros\ServerRequest; use Laminas\Diactoros\ServerRequest;
use Mezzio\Router\Middleware\ImplicitHeadMiddleware;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Prophecy\Argument; use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy; use Prophecy\Prophecy\ObjectProphecy;
@ -89,4 +91,23 @@ class RedirectActionTest extends TestCase
$handle->shouldHaveBeenCalledOnce(); $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();
}
} }