Added double check when parsing build epoch from the GeoLite db file in case it is not an integer

This commit is contained in:
Alejandro Celaya 2021-02-06 21:37:58 +01:00
parent 2f8ca6cf11
commit da65c05c4f
5 changed files with 103 additions and 19 deletions

View File

@ -7,18 +7,46 @@ namespace Shlinkio\Shlink\CLI\Exception;
use RuntimeException;
use Throwable;
use function sprintf;
class GeolocationDbUpdateFailedException extends RuntimeException implements ExceptionInterface
{
private bool $olderDbExists;
public static function create(bool $olderDbExists, ?Throwable $prev = null): self
public static function withOlderDb(?Throwable $prev = null): self
{
$e = new self(
'An error occurred while updating geolocation database, and an older version could not be found',
'An error occurred while updating geolocation database, but an older DB is already present.',
0,
$prev,
);
$e->olderDbExists = $olderDbExists;
$e->olderDbExists = true;
return $e;
}
public static function withoutOlderDb(?Throwable $prev = null): self
{
$e = new self(
'An error occurred while updating geolocation database, and an older version could not be found.',
0,
$prev,
);
$e->olderDbExists = false;
return $e;
}
/**
* @param mixed $buildEpoch
*/
public static function withInvalidEpochInOldDb($buildEpoch): self
{
$e = new self(sprintf(
'Build epoch with value "%s" from existing geolocation database, could not be parsed to integer.',
$buildEpoch,
));
$e->olderDbExists = true;
return $e;
}

View File

@ -6,11 +6,14 @@ namespace Shlinkio\Shlink\CLI\Util;
use Cake\Chronos\Chronos;
use GeoIp2\Database\Reader;
use MaxMind\Db\Reader\Metadata;
use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
use Symfony\Component\Lock\LockFactory;
use function is_int;
class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
{
private const LOCK_NAME = 'geolocation-db-update';
@ -52,7 +55,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
}
$meta = $this->geoLiteDbReader->metadata();
if ($this->buildIsTooOld($meta->buildEpoch)) {
if ($this->buildIsTooOld($meta)) {
$this->downloadNewDb(true, $mustBeUpdated, $handleProgress);
}
}
@ -69,14 +72,37 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
try {
$this->dbUpdater->downloadFreshCopy($handleProgress);
} catch (RuntimeException $e) {
throw GeolocationDbUpdateFailedException::create($olderDbExists, $e);
throw $olderDbExists
? GeolocationDbUpdateFailedException::withOlderDb($e)
: GeolocationDbUpdateFailedException::withoutOlderDb($e);
}
}
private function buildIsTooOld(int $buildTimestamp): bool
private function buildIsTooOld(Metadata $meta): bool
{
$buildTimestamp = $this->resolveBuildTimestamp($meta);
$buildDate = Chronos::createFromTimestamp($buildTimestamp);
$now = Chronos::now();
return $now->gt($buildDate->addDays(35));
}
private function resolveBuildTimestamp(Metadata $meta): int
{
// In theory the buildEpoch should be an int, but it has been reported to come as a string.
// See https://github.com/shlinkio/shlink/issues/1002 for context
/** @var int|string $buildEpoch */
$buildEpoch = $meta->buildEpoch;
if (is_int($buildEpoch)) {
return $buildEpoch;
}
$intBuildEpoch = (int) $buildEpoch;
if ($buildEpoch === (string) $intBuildEpoch) {
return $intBuildEpoch;
}
throw GeolocationDbUpdateFailedException::withInvalidEpochInOldDb($buildEpoch);
}
}

View File

@ -217,7 +217,9 @@ class LocateVisitsCommandTest extends TestCase
$mustBeUpdated($olderDbExists);
$handleProgress(100, 50);
throw GeolocationDbUpdateFailedException::create($olderDbExists);
throw $olderDbExists
? GeolocationDbUpdateFailedException::withOlderDb()
: GeolocationDbUpdateFailedException::withoutOlderDb();
},
);

View File

@ -14,26 +14,54 @@ class GeolocationDbUpdateFailedExceptionTest extends TestCase
{
/**
* @test
* @dataProvider provideCreateArgs
* @dataProvider providePrev
*/
public function createBuildsException(bool $olderDbExists, ?Throwable $prev): void
public function withOlderDbBuildsException(?Throwable $prev): void
{
$e = GeolocationDbUpdateFailedException::create($olderDbExists, $prev);
$e = GeolocationDbUpdateFailedException::withOlderDb($prev);
self::assertEquals($olderDbExists, $e->olderDbExists());
self::assertTrue($e->olderDbExists());
self::assertEquals(
'An error occurred while updating geolocation database, and an older version could not be found',
'An error occurred while updating geolocation database, but an older DB is already present.',
$e->getMessage(),
);
self::assertEquals(0, $e->getCode());
self::assertEquals($prev, $e->getPrevious());
}
public function provideCreateArgs(): iterable
/**
* @test
* @dataProvider providePrev
*/
public function withoutOlderDbBuildsException(?Throwable $prev): void
{
yield 'older DB and no prev' => [true, null];
yield 'older DB and prev' => [true, new RuntimeException('prev')];
yield 'no older DB and no prev' => [false, null];
yield 'no older DB and prev' => [false, new Exception('prev')];
$e = GeolocationDbUpdateFailedException::withoutOlderDb($prev);
self::assertFalse($e->olderDbExists());
self::assertEquals(
'An error occurred while updating geolocation database, and an older version could not be found.',
$e->getMessage(),
);
self::assertEquals(0, $e->getCode());
self::assertEquals($prev, $e->getPrevious());
}
public function providePrev(): iterable
{
yield 'no prev' => [null];
yield 'RuntimeException' => [new RuntimeException('prev')];
yield 'Exception' => [new Exception('prev')];
}
/** @test */
public function withInvalidEpochInOldDbBuildsException(): void
{
$e = GeolocationDbUpdateFailedException::withInvalidEpochInOldDb('foobar');
self::assertTrue($e->olderDbExists());
self::assertEquals(
'Build epoch with value "foobar" from existing geolocation database, could not be parsed to integer.',
$e->getMessage(),
);
}
}

View File

@ -169,7 +169,7 @@ class LocateShortUrlVisitTest extends TestCase
/** @test */
public function errorWhenUpdatingGeoLiteWithExistingCopyLogsWarning(): void
{
$e = GeolocationDbUpdateFailedException::create(true);
$e = GeolocationDbUpdateFailedException::withOlderDb();
$ipAddr = '1.2.3.0';
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$location = new Location('', '', '', '', 0.0, 0.0, '');
@ -200,7 +200,7 @@ class LocateShortUrlVisitTest extends TestCase
/** @test */
public function errorWhenDownloadingGeoLiteCancelsLocation(): void
{
$e = GeolocationDbUpdateFailedException::create(false);
$e = GeolocationDbUpdateFailedException::withoutOlderDb();
$ipAddr = '1.2.3.0';
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$location = new Location('', '', '', '', 0.0, 0.0, '');