From fa08014226c0c91d6b824390c214ca51cdb37a00 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 19 Nov 2024 09:08:04 +0100
Subject: [PATCH 1/2] Make sure IpGeolocationMiddleware skips localhost

---
 .../Geolocation/Middleware/IpGeolocationMiddleware.php |  5 ++++-
 module/Core/src/Visit/Geolocation/VisitLocator.php     |  2 +-
 .../Middleware/IpGeolocationMiddlewareTest.php         | 10 ++++++++--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php b/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php
index 4e2e533b0..f5657e648 100644
--- a/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php
+++ b/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php
@@ -9,6 +9,7 @@
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
+use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Config\Options\TrackingOptions;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
@@ -46,7 +47,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
     private function geolocateIpAddress(string|null $ipAddress): Location
     {
         try {
-            return $ipAddress === null ? Location::empty() : $this->ipLocationResolver->resolveIpLocation($ipAddress);
+            return $ipAddress === null || $ipAddress === IpAddress::LOCALHOST
+                ? Location::empty()
+                : $this->ipLocationResolver->resolveIpLocation($ipAddress);
         } catch (WrongIpException $e) {
             $this->logger->warning('Tried to locate IP address, but it seems to be wrong. {e}', ['e' => $e]);
             return Location::empty();
diff --git a/module/Core/src/Visit/Geolocation/VisitLocator.php b/module/Core/src/Visit/Geolocation/VisitLocator.php
index f3aba1931..8f69ba2c5 100644
--- a/module/Core/src/Visit/Geolocation/VisitLocator.php
+++ b/module/Core/src/Visit/Geolocation/VisitLocator.php
@@ -54,7 +54,7 @@ private function locateVisits(iterable $results, VisitGeolocationHelperInterface
                 }
 
                 // If the IP address is non-locatable, locate it as empty to prevent next processes to pick it again
-                $location = Location::emptyInstance();
+                $location = Location::empty();
             }
 
             $this->locateVisit($visit, VisitLocation::fromGeolocation($location), $helper);
diff --git a/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php b/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php
index f203fb855..80768f5bf 100644
--- a/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php
+++ b/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php
@@ -15,6 +15,7 @@
 use Psr\Log\LoggerInterface;
 use RuntimeException;
 use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory;
+use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Config\Options\TrackingOptions;
 use Shlinkio\Shlink\Core\Geolocation\Middleware\IpGeolocationMiddleware;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
@@ -67,13 +68,18 @@ public function warningIsLoggedIfGeoLiteDbDoesNotExist(): void
     }
 
     #[Test]
-    public function emptyLocationIsReturnedIfIpAddressDoesNotExistInRequest(): void
+    #[TestWith([null])]
+    #[TestWith([IpAddress::LOCALHOST])]
+    public function emptyLocationIsReturnedIfIpAddressIsNotLocatable(string|null $ipAddress): void
     {
         $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true);
         $this->ipLocationResolver->expects($this->never())->method('resolveIpLocation');
         $this->logger->expects($this->never())->method('warning');
 
-        $request = ServerRequestFactory::fromGlobals();
+        $request = ServerRequestFactory::fromGlobals()->withAttribute(
+            IpAddressMiddlewareFactory::REQUEST_ATTR,
+            $ipAddress,
+        );
         $this->handler->expects($this->once())->method('handle')->with($this->callback(
             function (ServerRequestInterface $req): bool {
                 $location = $req->getAttribute(Location::class);

From f57f159002c19a76569e49e5d86a32485ef15155 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 19 Nov 2024 09:10:47 +0100
Subject: [PATCH 2/2] Remove no longer used Visit::isLocatable method

---
 module/Core/src/Visit/Entity/Visit.php | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php
index e26d5f802..9e8540bc5 100644
--- a/module/Core/src/Visit/Entity/Visit.php
+++ b/module/Core/src/Visit/Entity/Visit.php
@@ -127,11 +127,6 @@ public function getVisitLocation(): VisitLocation|null
         return $this->visitLocation;
     }
 
-    public function isLocatable(): bool
-    {
-        return $this->hasRemoteAddr() && $this->remoteAddr !== IpAddress::LOCALHOST;
-    }
-
     public function locate(VisitLocation $visitLocation): self
     {
         $this->visitLocation = $visitLocation;