Merge pull request #169 from codisart/bug-135-fix-GPS-point-validator

Fix #135: better GPS point validator detection of invalid coordinate numeric conversions
laminas · Feb 22, 2024 · e77d1ca · e77d1ca
2 parents 5c3fc8c + 4923b71
commit e77d1ca
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 43 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1513,10 +1513,6 @@
     </RedundantConditionGivenDocblockType>
   </file>
   <file src="src/GpsPoint.php">
-    <DocblockTypeContradiction>
-      <code>$value === false || $value === null</code>
-      <code>$value === null</code>
-    </DocblockTypeContradiction>
     <InvalidOperand>
       <code>$matches[1][0]</code>
       <code>$matches[2][0]</code>

diff --git a/src/GpsPoint.php b/src/GpsPoint.php
@@ -3,6 +3,7 @@
 namespace Laminas\Validator;
 
 use function explode;
+use function is_numeric;
 use function preg_match;
 use function preg_match_all;
 use function preg_replace;
@@ -15,11 +16,10 @@ final class GpsPoint extends AbstractValidator
     public const CONVERT_ERROR         = 'gpsPointConvertError';
     public const INCOMPLETE_COORDINATE = 'gpsPointIncompleteCoordinate';
 
-    /** @var array */
-    protected $messageTemplates = [
-        'gpsPointOutOfBounds'          => '%value% is out of Bounds.',
-        'gpsPointConvertError'         => '%value% can not converted into a Decimal Degree Value.',
-        'gpsPointIncompleteCoordinate' => '%value% did not provided a complete Coordinate',
+    protected array $messageTemplates = [
+        self::OUT_OF_BOUNDS         => '%value% is out of Bounds.',
+        self::CONVERT_ERROR         => '%value% can not converted into a Decimal Degree Value.',
+        self::INCOMPLETE_COORDINATE => '%value% did not provided a complete Coordinate',
     ];
 
     /**
@@ -29,11 +29,9 @@ final class GpsPoint extends AbstractValidator
      * getMessages() will return an array of messages that explain why the
      * validation failed.
      *
-     * @param  mixed $value
-     * @return bool
      * @throws Exception\RuntimeException If validation of $value is impossible.
      */
-    public function isValid($value)
+    public function isValid(mixed $value): bool
     {
         if (! str_contains($value, ',')) {
             $this->error(self::INCOMPLETE_COORDINATE, $value);
@@ -42,17 +40,10 @@ public function isValid($value)
 
         [$lat, $long] = explode(',', $value);
 
-        if ($this->isValidCoordinate($lat, 90.0000) && $this->isValidCoordinate($long, 180.000)) {
-            return true;
-        }
-
-        return false;
+        return $this->isValidCoordinate($lat, 90.0000) && $this->isValidCoordinate($long, 180.000);
     }
 
-    /**
-     * @param string $value
-     */
-    private function isValidCoordinate($value, float $maxBoundary): bool
+    private function isValidCoordinate(string $value, float $maxBoundary): bool
     {
         $this->value = $value;
 
@@ -63,19 +54,23 @@ private function isValidCoordinate($value, float $maxBoundary): bool
             $value = $this->removeDegreeSign($value);
         }
 
-        if ($value === false || $value === null) {
+        if ($value === false) {
             $this->error(self::CONVERT_ERROR);
             return false;
         }
 
-        $doubleLatitude = (double) $value;
+        $castedValue = (float) $value;
+        if (! is_numeric($value) && $castedValue === 0.0) {
+            $this->error(self::CONVERT_ERROR);
+            return false;
+        }
 
-        if ($doubleLatitude <= $maxBoundary && $doubleLatitude >= $maxBoundary * -1) {
-            return true;
+        if (! $this->isValueInbound($castedValue, $maxBoundary)) {
+            $this->error(self::OUT_OF_BOUNDS);
+            return false;
         }
 
-        $this->error(self::OUT_OF_BOUNDS);
-        return false;
+        return true;
     }
 
     /**
@@ -86,11 +81,7 @@ private function isDMSValue(string $value): bool
         return preg_match('/([°\'"]+[NESW])/', $value) > 0;
     }
 
-    /**
-     * @param string $value
-     * @return false|float
-     */
-    private function convertValue($value)
+    private function convertValue(string $value): false|float
     {
         $matches = [];
         $result  = preg_match_all('/(\d{1,3})°(\d{1,2})\'(\d{1,2}[\.\d]{0,6})"[NESW]/i', $value, $matches);
@@ -99,24 +90,23 @@ private function convertValue($value)
             return false;
         }
 
-        return $matches[1][0] + $matches[2][0] / 60 + ((double) $matches[3][0]) / 3600;
+        return $matches[1][0] + $matches[2][0] / 60 + ((float) $matches[3][0]) / 3600;
     }
 
-    /**
-     * @param string $value
-     * @return string
-     */
-    private function removeWhiteSpace($value)
+    private function removeWhiteSpace(string $value): string
     {
         return preg_replace('/\s/', '', $value);
     }
 
-    /**
-     * @param string $value
-     * @return string
-     */
-    private function removeDegreeSign($value)
+    private function removeDegreeSign(string $value): string
     {
         return str_replace('°', '', $value);
     }
+
+    private function isValueInbound(float $value, float $boundary): bool
+    {
+        $max = $boundary;
+        $min = -1 * $boundary;
+        return $min <= $value && $value <= $max;
+    }
 }
diff --git a/test/GPSPointTest.php b/test/GPSPointTest.php
@@ -74,8 +74,11 @@ public static function errorMessageTestValues(): array
     {
         return [
             ['63 47 24.691 N, 18 2 54.363 W', GpsPoint::OUT_OF_BOUNDS, '63 47 24.691 N'],
+            ['65° 4\' N,-22.728867530822754', GpsPoint::CONVERT_ERROR, '65° 4\' N'],
             ['° \' " N,° \' " E', GpsPoint::CONVERT_ERROR, '° \' " N'],
             ['° \' " N', GpsPoint::INCOMPLETE_COORDINATE, '° \' " N'],
+            ['foo,bar', GpsPoint::CONVERT_ERROR, 'foo'],
+            [' ,-22.4', GpsPoint::CONVERT_ERROR, ' '],
         ];
     }
 }