diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 8aa3ddb7..a86a98c4 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -85,7 +85,7 @@ class RedirectAction implements MiddlewareInterface } // Track visit to this short code - $this->visitTracker->track($shortCode); + $this->visitTracker->track($shortCode, $request); // Return a redirect response to the long URL. // Use a temporary redirect to make sure browsers always hit the server for analytics purposes diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 87187aad..94647086 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -3,6 +3,7 @@ namespace Shlinkio\Shlink\Core\Service; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Doctrine\ORM\EntityManagerInterface; +use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -31,12 +32,10 @@ class VisitsTracker implements VisitsTrackerInterface * Tracks a new visit to provided short code, using an array of data to look up information * * @param string $shortCode - * @param array $visitorData Defaults to global $_SERVER + * @param ServerRequestInterface $request */ - public function track($shortCode, array $visitorData = null) + public function track($shortCode, ServerRequestInterface $request) { - $visitorData = $visitorData ?: $_SERVER; - /** @var ShortUrl $shortUrl */ $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ 'shortCode' => $shortCode, @@ -44,22 +43,27 @@ class VisitsTracker implements VisitsTrackerInterface $visit = new Visit(); $visit->setShortUrl($shortUrl) - ->setUserAgent($this->getArrayValue($visitorData, 'HTTP_USER_AGENT')) - ->setReferer($this->getArrayValue($visitorData, 'HTTP_REFERER')) - ->setRemoteAddr($this->getArrayValue($visitorData, 'REMOTE_ADDR')); + ->setUserAgent($request->getHeaderLine('User-Agent')) + ->setReferer($request->getHeaderLine('Referer')) + ->setRemoteAddr($this->findOutRemoteAddr($request)); $this->em->persist($visit); $this->em->flush(); } /** - * @param array $array - * @param $key - * @param null $default - * @return mixed|null + * @param ServerRequestInterface $request + * @return string */ - protected function getArrayValue(array $array, $key, $default = null) + protected function findOutRemoteAddr(ServerRequestInterface $request) { - return isset($array[$key]) ? $array[$key] : $default; + $forwardedFor = $request->getHeaderLine('X-Forwarded-For'); + if (empty($forwardedFor)) { + $serverParams = $request->getServerParams(); + return isset($serverParams['REMOTE_ADDR']) ? $serverParams['REMOTE_ADDR'] : null; + } + + $ips = explode(',', $forwardedFor); + return $ips[0]; } /** diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index cec254d3..6aecabe4 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -1,6 +1,7 @@ em->persist(Argument::any())->shouldBeCalledTimes(1); $this->em->flush()->shouldBeCalledTimes(1); - $this->visitsTracker->track($shortCode); + $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals()); + } + + /** + * @test + */ + public function trackUsesForwardedForHeaderIfPresent() + { + $shortCode = '123ABC'; + $test = $this; + $repo = $this->prophesize(EntityRepository::class); + $repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl()); + + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); + $this->em->persist(Argument::any())->will(function ($args) use ($test) { + /** @var Visit $visit */ + $visit = $args[0]; + $test->assertEquals('4.3.2.1', $visit->getRemoteAddr()); + })->shouldBeCalledTimes(1); + $this->em->flush()->shouldBeCalledTimes(1); + + $this->visitsTracker->track($shortCode, ServerRequestFactory::fromGlobals( + ['REMOTE_ADDR' => '1.2.3.4'] + )->withHeader('X-Forwarded-For', '4.3.2.1,99.99.99.99')); } /**