diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index fb9be0c4..34ef9e78 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -15,7 +15,9 @@ return [ ], 'dependencies' => [ - EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class, + 'factories' => [ + EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class, + ], ], ConfigAbstractFactory::class => [ diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php index defa0bf8..fa6aaa09 100644 --- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php +++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php @@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Core\EventDispatcher; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Core\Entity\Visit; @@ -42,9 +43,17 @@ class LocateShortUrlVisit return; } - $location = $visit->isLocatable() - ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr()) - : Location::emptyInstance(); + try { + $location = $visit->isLocatable() + ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr()) + : Location::emptyInstance(); + } catch (WrongIpException $e) { + $this->logger->warning( + sprintf('Tried to locate visit with id "%s", but its address seems to be wrong. {e}', $visitId), + ['e' => $e] + ); + return; + } $visit->locate(new VisitLocation($location)); $this->em->flush($visit); diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php index 7dfaacf0..b40ede09 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; use Shlinkio\Shlink\Common\Util\IpAddress; @@ -57,6 +58,29 @@ class LocateShortUrlVisitTest extends TestCase $logWarning->shouldHaveBeenCalled(); } + /** @test */ + public function invalidAddressLogsWarning(): void + { + $event = new ShortUrlVisited('123'); + $findVisit = $this->em->find(Visit::class, '123')->willReturn( + new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4')) + ); + $resolveLocation = $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->willThrow( + WrongIpException::class + ); + $logWarning = $this->logger->warning( + Argument::containingString('Tried to locate visit with id "123", but its address seems to be wrong.'), + Argument::type('array') + ); + + ($this->locateVisit)($event); + + $findVisit->shouldHaveBeenCalledOnce(); + $resolveLocation->shouldHaveBeenCalledOnce(); + $logWarning->shouldHaveBeenCalled(); + $this->em->flush(Argument::cetera())->shouldNotHaveBeenCalled(); + } + /** * @test * @dataProvider provideNonLocatableVisits diff --git a/phpstan.neon b/phpstan.neon index 96f5d0c3..7756bc14 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,3 +2,4 @@ parameters: ignoreErrors: - '#League\\Plates\\callback#' - '#is not subtype of Throwable#' + - '#ObjectManager::flush()#'