Merge pull request #2272 from acelaya-forks/feature/geolocate-localhost-fix

Make sure IpGeolocationMiddleware skips localhost
This commit is contained in:
Alejandro Celaya 2024-11-19 09:14:45 +01:00 committed by GitHub
commit c323bfcd63
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 13 additions and 9 deletions

View File

@ -9,6 +9,7 @@ use Psr\Http\Message\ServerRequestInterface;
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 @@ readonly class IpGeolocationMiddleware implements MiddlewareInterface
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();

View File

@ -127,11 +127,6 @@ class Visit extends AbstractEntity implements JsonSerializable
return $this->visitLocation;
}
public function isLocatable(): bool
{
return $this->hasRemoteAddr() && $this->remoteAddr !== IpAddress::LOCALHOST;
}
public function locate(VisitLocation $visitLocation): self
{
$this->visitLocation = $visitLocation;

View File

@ -54,7 +54,7 @@ readonly class VisitLocator implements VisitLocatorInterface
}
// 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);

View File

@ -15,6 +15,7 @@ use Psr\Http\Server\RequestHandlerInterface;
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 @@ class IpGeolocationMiddlewareTest extends TestCase
}
#[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);