diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php
index b858546d..2ae4e432 100644
--- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php
@@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
use Shlinkio\Shlink\Common\Service\IpLocationResolverInterface;
+use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Service\VisitServiceInterface;
use Symfony\Component\Console\Command\Command;
@@ -15,7 +16,6 @@ use Zend\I18n\Translator\TranslatorInterface;
class ProcessVisitsCommand extends Command
{
- private const LOCALHOST = '127.0.0.1';
public const NAME = 'visit:process';
/**
@@ -59,7 +59,7 @@ class ProcessVisitsCommand extends Command
foreach ($visits as $visit) {
$ipAddr = $visit->getRemoteAddr();
$io->write(\sprintf('%s %s', $this->translator->translate('Processing IP'), $ipAddr));
- if ($ipAddr === self::LOCALHOST) {
+ if ($ipAddr === IpAddress::LOCALHOST) {
$io->writeln(
\sprintf(' (%s)', $this->translator->translate('Ignored localhost address'))
);
diff --git a/module/CLI/test/Command/Shortcode/GetVisitsCommandTest.php b/module/CLI/test/Command/Shortcode/GetVisitsCommandTest.php
index d80d6d4f..4a647814 100644
--- a/module/CLI/test/Command/Shortcode/GetVisitsCommandTest.php
+++ b/module/CLI/test/Command/Shortcode/GetVisitsCommandTest.php
@@ -86,8 +86,8 @@ class GetVisitsCommandTest extends TestCase
'shortCode' => $shortCode,
]);
$output = $this->commandTester->getDisplay();
- $this->assertTrue(strpos($output, 'foo') > 0);
- $this->assertTrue(strpos($output, '1.2.3.4') > 0);
- $this->assertTrue(strpos($output, 'bar') > 0);
+ $this->assertTrue(\strpos($output, 'foo') > 0);
+ $this->assertTrue(\strpos($output, '1.2.3.0') > 0);
+ $this->assertTrue(\strpos($output, 'bar') > 0);
}
}
diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php
index 5f843fa4..174ce3fb 100644
--- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php
+++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php
@@ -67,9 +67,9 @@ class ProcessVisitsCommandTest extends TestCase
'command' => 'visit:process',
]);
$output = $this->commandTester->getDisplay();
- $this->assertTrue(strpos($output, 'Processing IP 1.2.3.4') === 0);
- $this->assertTrue(strpos($output, 'Processing IP 4.3.2.1') > 0);
- $this->assertTrue(strpos($output, 'Processing IP 12.34.56.78') > 0);
+ $this->assertEquals(0, \strpos($output, 'Processing IP 1.2.3.0'));
+ $this->assertGreaterThan(0, \strpos($output, 'Processing IP 4.3.2.0'));
+ $this->assertGreaterThan(0, \strpos($output, 'Processing IP 12.34.56.0'));
}
/**
@@ -87,15 +87,15 @@ class ProcessVisitsCommandTest extends TestCase
$this->visitService->getUnlocatedVisits()->willReturn($visits)
->shouldBeCalledTimes(1);
- $this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits) - 2);
+ $this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(\count($visits) - 2);
$this->ipResolver->resolveIpLocation(Argument::any())->willReturn([])
- ->shouldBeCalledTimes(count($visits) - 2);
+ ->shouldBeCalledTimes(\count($visits) - 2);
$this->commandTester->execute([
'command' => 'visit:process',
]);
$output = $this->commandTester->getDisplay();
- $this->assertTrue(strpos($output, 'Ignored localhost address') > 0);
+ $this->assertGreaterThan(0, \strpos($output, 'Ignored localhost address'));
}
/**
diff --git a/module/Common/src/Exception/WrongIpException.php b/module/Common/src/Exception/WrongIpException.php
index 837f9ec7..d31a2436 100644
--- a/module/Common/src/Exception/WrongIpException.php
+++ b/module/Common/src/Exception/WrongIpException.php
@@ -5,8 +5,8 @@ namespace Shlinkio\Shlink\Common\Exception;
class WrongIpException extends RuntimeException
{
- public static function fromIpAddress($ipAddress, \Throwable $prev = null)
+ public static function fromIpAddress($ipAddress, \Throwable $prev = null): self
{
- return new self(sprintf('Provided IP "%s" is invalid', $ipAddress), 0, $prev);
+ return new self(\sprintf('Provided IP "%s" is invalid', $ipAddress), 0, $prev);
}
}
diff --git a/module/Common/src/Util/IpAddress.php b/module/Common/src/Util/IpAddress.php
new file mode 100644
index 00000000..1671038f
--- /dev/null
+++ b/module/Common/src/Util/IpAddress.php
@@ -0,0 +1,74 @@
+isLocalhost = $address === self::LOCALHOST;
+ [$instance->firstOctet, $instance->secondOctet, $instance->thirdOctet, $instance->fourthOctet] = $parts;
+ return $instance;
+ }
+
+ public function getObfuscatedCopy(): self
+ {
+ $copy = clone $this;
+ $copy->fourthOctet = $this->isLocalhost ? $this->fourthOctet : self::OBFUSCATED_OCTET;
+ return $copy;
+ }
+
+ public function __toString(): string
+ {
+ return \implode('.', [
+ $this->firstOctet,
+ $this->secondOctet,
+ $this->thirdOctet,
+ $this->fourthOctet,
+ ]);
+ }
+}
diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php
index 48edfb11..24346406 100644
--- a/module/Core/src/Entity/Visit.php
+++ b/module/Core/src/Entity/Visit.php
@@ -5,6 +5,8 @@ namespace Shlinkio\Shlink\Core\Entity;
use Doctrine\ORM\Mapping as ORM;
use Shlinkio\Shlink\Common\Entity\AbstractEntity;
+use Shlinkio\Shlink\Common\Exception\WrongIpException;
+use Shlinkio\Shlink\Common\Util\IpAddress;
/**
* Class Visit
@@ -54,109 +56,81 @@ class Visit extends AbstractEntity implements \JsonSerializable
$this->date = new \DateTime();
}
- /**
- * @return string
- */
public function getReferer(): string
{
return $this->referer;
}
- /**
- * @param string $referer
- * @return $this
- */
- public function setReferer($referer): self
+ public function setReferer(string $referer): self
{
$this->referer = $referer;
return $this;
}
- /**
- * @return \DateTime
- */
public function getDate(): \DateTime
{
return $this->date;
}
- /**
- * @param \DateTime $date
- * @return $this
- */
- public function setDate($date): self
+ public function setDate(\DateTime $date): self
{
$this->date = $date;
return $this;
}
- /**
- * @return ShortUrl
- */
public function getShortUrl(): ShortUrl
{
return $this->shortUrl;
}
- /**
- * @param ShortUrl $shortUrl
- * @return $this
- */
- public function setShortUrl($shortUrl): self
+ public function setShortUrl(ShortUrl $shortUrl): self
{
$this->shortUrl = $shortUrl;
return $this;
}
- /**
- * @return string
- */
public function getRemoteAddr(): string
{
return $this->remoteAddr;
}
- /**
- * @param string $remoteAddr
- * @return $this
- */
- public function setRemoteAddr($remoteAddr): self
+ public function setRemoteAddr(?string $remoteAddr): self
{
- $this->remoteAddr = $remoteAddr;
+ // TODO Generate hash for the original remote address
+ $this->remoteAddr = $this->obfuscateAddress($remoteAddr);
return $this;
}
- /**
- * @return string
- */
+ private function obfuscateAddress(?string $address): ?string
+ {
+ if ($address === null) {
+ return null;
+ }
+
+ try {
+ return (string) IpAddress::fromString($address)->getObfuscatedCopy();
+ } catch (WrongIpException $e) {
+ return null;
+ }
+ }
+
public function getUserAgent(): string
{
return $this->userAgent;
}
- /**
- * @param string $userAgent
- * @return $this
- */
- public function setUserAgent($userAgent): self
+ public function setUserAgent(string $userAgent): self
{
$this->userAgent = $userAgent;
return $this;
}
- /**
- * @return VisitLocation
- */
public function getVisitLocation(): VisitLocation
{
return $this->visitLocation;
}
- /**
- * @param VisitLocation $visitLocation
- * @return $this
- */
- public function setVisitLocation($visitLocation): self
+ public function setVisitLocation(VisitLocation $visitLocation): self
{
$this->visitLocation = $visitLocation;
return $this;
diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php
index 2103c04a..bc2a4305 100644
--- a/module/Core/src/Service/VisitsTracker.php
+++ b/module/Core/src/Service/VisitsTracker.php
@@ -31,7 +31,7 @@ class VisitsTracker implements VisitsTrackerInterface
* @throws ORM\ORMInvalidArgumentException
* @throws ORM\OptimisticLockException
*/
- public function track($shortCode, ServerRequestInterface $request)
+ public function track($shortCode, ServerRequestInterface $request): void
{
/** @var ShortUrl $shortUrl */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([
@@ -52,9 +52,8 @@ class VisitsTracker implements VisitsTrackerInterface
/**
* @param ServerRequestInterface $request
- * @return string|null
*/
- private function findOutRemoteAddr(ServerRequestInterface $request)
+ private function findOutRemoteAddr(ServerRequestInterface $request): ?string
{
$forwardedFor = $request->getHeaderLine('X-Forwarded-For');
if (empty($forwardedFor)) {
@@ -62,7 +61,7 @@ class VisitsTracker implements VisitsTrackerInterface
return $serverParams['REMOTE_ADDR'] ?? null;
}
- $ips = explode(',', $forwardedFor);
+ $ips = \explode(',', $forwardedFor);
return $ips[0] ?? null;
}
diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php
index 802983b0..75bc8d6f 100644
--- a/module/Core/src/Service/VisitsTrackerInterface.php
+++ b/module/Core/src/Service/VisitsTrackerInterface.php
@@ -16,7 +16,7 @@ interface VisitsTrackerInterface
* @param string $shortCode
* @param ServerRequestInterface $request
*/
- public function track($shortCode, ServerRequestInterface $request);
+ public function track($shortCode, ServerRequestInterface $request): void;
/**
* Returns the visits on certain short code
diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php
index f827b0d1..ea1f42e1 100644
--- a/module/Core/test/Action/PixelActionTest.php
+++ b/module/Core/test/Action/PixelActionTest.php
@@ -52,8 +52,7 @@ class PixelActionTest extends TestCase
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn(
(new ShortUrl())->setLongUrl('http://domain.com/foo/bar')
)->shouldBeCalledTimes(1);
- $this->visitTracker->track(Argument::cetera())->willReturn(null)
- ->shouldBeCalledTimes(1);
+ $this->visitTracker->track(Argument::cetera())->shouldBeCalledTimes(1);
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
$response = $this->action->process($request, TestUtils::createReqHandlerMock()->reveal());
diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php
index 0c83f8fe..27de2a7e 100644
--- a/module/Core/test/Action/RedirectActionTest.php
+++ b/module/Core/test/Action/RedirectActionTest.php
@@ -55,8 +55,7 @@ class RedirectActionTest extends TestCase
$shortUrl = (new ShortUrl())->setLongUrl($expectedUrl);
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)
->shouldBeCalledTimes(1);
- $this->visitTracker->track(Argument::cetera())->willReturn(null)
- ->shouldBeCalledTimes(1);
+ $this->visitTracker->track(Argument::cetera())->shouldBeCalledTimes(1);
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode);
$response = $this->action->process($request, TestUtils::createReqHandlerMock()->reveal());
@@ -75,8 +74,7 @@ class RedirectActionTest extends TestCase
$shortCode = 'abc123';
$this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class)
->shouldBeCalledTimes(1);
- $this->visitTracker->track(Argument::cetera())->willReturn(null)
- ->shouldNotBeCalled();
+ $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
$delegate = $this->prophesize(RequestHandlerInterface::class);
/** @var MethodProphecy $process */
@@ -98,8 +96,7 @@ class RedirectActionTest extends TestCase
$shortUrl = (new ShortUrl())->setLongUrl($expectedUrl);
$this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)
->shouldBeCalledTimes(1);
- $this->visitTracker->track(Argument::cetera())->willReturn(null)
- ->shouldNotBeCalled();
+ $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode)
->withQueryParams(['foobar' => true]);
diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php
index f931c4e4..f9703e3b 100644
--- a/module/Core/test/Service/VisitsTrackerTest.php
+++ b/module/Core/test/Service/VisitsTrackerTest.php
@@ -61,7 +61,7 @@ class VisitsTrackerTest extends TestCase
$this->em->persist(Argument::any())->will(function ($args) use ($test) {
/** @var Visit $visit */
$visit = $args[0];
- $test->assertEquals('4.3.2.1', $visit->getRemoteAddr());
+ $test->assertEquals('4.3.2.0', $visit->getRemoteAddr());
})->shouldBeCalledTimes(1);
$this->em->flush(Argument::type(Visit::class))->shouldBeCalledTimes(1);