From 97e5c828c570342fe20a96c82e31bb5a2129bf45 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 15 Oct 2026 22:37:59 +1300 Subject: [PATCH 1/2] fix(validator): validate the geometry of a spatial filter Filter only asked a spatial filter's value to be an array. A polygon nested too deep or a point out of range reached the engine and failed there, which MySQL raised as error 3037, 3616 or 3617 and nothing mapped, and a point of strings was cast to (0, 0) by the builder and silently matched against the origin. The Spatial validator of stored values read an object's coordinates by position and raised an undefined key warning. 7.4.2 checks both the same way. A spatial filter's geometry, and a distance query's unless it is WKT, is now read in the shape the builders read it, a point, a linestring or a polygon, and held to the rules of a stored value of that type: numeric coordinates in longitude and latitude range, and closed rings of at least four points. Spatial refuses a coordinate, point or ring given as an object, and the GIS errors map to Exception\Query: ER_GIS_INVALID_DATA on both engines, and MySQL's longitude and latitude range errors on MySQL only, since MariaDB reads such a geometry as NULL. Two E2E queries leaned on the loose check: a linestring wrapped one level too deep, which the builder read as a polygon of one two-point ring, and a point at latitude 100. They now pass well-formed geometry. Co-Authored-By: Claude Opus 5.5 --- src/Database/Adapter/MariaDB.php | 9 ++ src/Database/Adapter/MySQL.php | 5 + src/Database/Validator/Geometry.php | 59 +++++++++ src/Database/Validator/Query/Filter.php | 24 +++- src/Database/Validator/Spatial.php | 10 +- .../e2e/Adapter/Scopes/RelationshipTests.php | 4 +- tests/e2e/Adapter/Scopes/SpatialTests.php | 10 +- tests/unit/EngineErrorMappingTest.php | 27 +++++ .../Validator/Query/FilterGeometryTest.php | 113 ++++++++++++++++++ 9 files changed, 247 insertions(+), 14 deletions(-) create mode 100644 src/Database/Validator/Geometry.php create mode 100644 tests/unit/Validator/Query/FilterGeometryTest.php diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 140b24864..78a09bbe5 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1045,6 +1045,11 @@ protected function decodePolygon(string $wkb): array private const string TIMEOUT_SETTING = 'timeout'; + /** + * ER_GIS_INVALID_DATA. MariaDB reads most invalid geometries as NULL instead. + */ + protected const array INVALID_GEOMETRY_ERRORS = [3037]; + /** The session timeout last set, in milliseconds. */ private int $appliedTimeout = 0; @@ -1609,6 +1614,10 @@ protected function processException(PDOException $e): Exception return new LimitException('Value is out of range', $e->getCode(), $e); } + if (isset($e->errorInfo[1]) && \in_array($e->errorInfo[1], static::INVALID_GEOMETRY_ERRORS, true)) { + return new QueryException('Invalid spatial value', $e->getCode(), $e); + } + if ($e->getCode() === '42000' && isset($e->errorInfo[1]) && $e->errorInfo[1] === 1049) { return new NotFoundException('Database not found', $e->getCode(), $e); } diff --git a/src/Database/Adapter/MySQL.php b/src/Database/Adapter/MySQL.php index d024ade55..2350b8d0e 100644 --- a/src/Database/Adapter/MySQL.php +++ b/src/Database/Adapter/MySQL.php @@ -117,6 +117,11 @@ protected function processException(PDOException $e): Exception return parent::processException($e); } + /** + * ER_GIS_INVALID_DATA, ER_LONGITUDE_OUT_OF_RANGE and ER_LATITUDE_OUT_OF_RANGE. + */ + protected const array INVALID_GEOMETRY_ERRORS = [3037, 3616, 3617]; + #[\Override] protected function dialectBuilder(): SQLBuilder&Scoping { diff --git a/src/Database/Validator/Geometry.php b/src/Database/Validator/Geometry.php new file mode 100644 index 000000000..72b2bca20 --- /dev/null +++ b/src/Database/Validator/Geometry.php @@ -0,0 +1,59 @@ +message; + } + + #[\Override] + public function isValid(mixed $value): bool + { + if (! \is_array($value) || $value === [] || ! \array_is_list($value)) { + return false; + } + + $first = $value[0]; + [$type, $geometry] = match (true) { + ! \is_array($first) => [ColumnType::Point, $value], + \is_array($first[0] ?? null) => [ColumnType::Polygon, $value], + \count($value) === 1 => [ColumnType::Point, $first], + default => [ColumnType::Linestring, $value], + }; + + $spatial = new Spatial($type->value); + if (! $spatial->isValid($geometry)) { + $this->message = $spatial->getDescription(); + + return false; + } + + return true; + } + + #[\Override] + public function isArray(): bool + { + return false; + } + + #[\Override] + public function getType(): string + { + return self::TYPE_ARRAY; + } +} diff --git a/src/Database/Validator/Query/Filter.php b/src/Database/Validator/Query/Filter.php index a2fb73359..490d04b29 100644 --- a/src/Database/Validator/Query/Filter.php +++ b/src/Database/Validator/Query/Filter.php @@ -8,10 +8,12 @@ use Utopia\Database\Query; use Utopia\Database\Validator\BigInt; use Utopia\Database\Validator\Datetime as DatetimeValidator; +use Utopia\Database\Validator\Geometry; use Utopia\Database\Validator\ObjectPath; use Utopia\Database\Validator\Query\Joined\Attributes; use Utopia\Database\Validator\Query\Joined\Collection; use Utopia\Database\Validator\Sequence; +use Utopia\Database\Validator\Spatial; use Utopia\Query\Method; use Utopia\Query\Query as BaseQuery; use Utopia\Query\Schema\ColumnType; @@ -29,6 +31,8 @@ class Filter extends Base */ protected array $schema = []; + private const array DISTANCE_METHODS = [Method::DistanceEqual, Method::DistanceNotEqual, Method::DistanceGreaterThan, Method::DistanceLessThan]; + /** * @param array $attributes */ @@ -297,6 +301,8 @@ protected function isValidAttributeAndValues(string $attribute, array $values, M } } + $geometry = new Geometry(); + foreach ($values as $value) { switch ($attributeType) { case ColumnType::Object: @@ -323,6 +329,12 @@ protected function isValidAttributeAndValues(string $attribute, array $values, M return false; } + if (! \in_array($method, self::DISTANCE_METHODS, true) && ! $geometry->isValid($value)) { + $this->message = 'Invalid spatial data for attribute "'.$attribute.'": '.$geometry->getDescription(); + + return false; + } + continue 2; case ColumnType::Vector: @@ -437,7 +449,8 @@ protected function isEmpty(array $values): bool } /** - * A distance query holds one `[geometry, distance, meters]` tuple: a finite distance and whether it is in meters. + * A distance query holds one `[geometry, distance, meters]` tuple: the geometry as coordinates or WKT, a finite + * distance and whether it is in meters. * * @param array $values */ @@ -450,7 +463,14 @@ private function isValidDistance(array $values): bool return false; } - [, $distance, $meters] = $tuple; + [$geometry, $distance, $meters] = $tuple; + + $validator = new Geometry(); + if (\is_string($geometry) ? ! Spatial::isWktString($geometry) : ! $validator->isValid($geometry)) { + $this->message = 'Distance query requires a geometry as coordinates or WKT: '.$validator->getDescription(); + + return false; + } if (! \is_numeric($distance) || ! \is_finite((float) $distance)) { $this->message = 'Distance query requires a finite distance'; diff --git a/src/Database/Validator/Spatial.php b/src/Database/Validator/Spatial.php index eaa3be1e4..7c854a8f3 100644 --- a/src/Database/Validator/Spatial.php +++ b/src/Database/Validator/Spatial.php @@ -26,7 +26,7 @@ public function __construct(string $spatialType) */ protected function validatePoint(array $value): bool { - if (count($value) !== 2) { + if (count($value) !== 2 || ! array_is_list($value)) { $this->message = 'Point must be an array of two numeric values [x, y]'; return false; @@ -55,7 +55,7 @@ protected function validateLineString(array $value): bool } foreach ($value as $pointIndex => $point) { - if (! is_array($point) || count($point) !== 2) { + if (! is_array($point) || count($point) !== 2 || ! array_is_list($point)) { $this->message = 'Each point in LineString must be an array of two values [x, y]'; return false; @@ -91,7 +91,7 @@ protected function validatePolygon(array $value): bool } $isSingleRing = isset($value[0]) && is_array($value[0]) && - count($value[0]) === 2 && + count($value[0]) === 2 && array_is_list($value[0]) && is_numeric($value[0][0]) && is_numeric($value[0][1]); @@ -100,7 +100,7 @@ protected function validatePolygon(array $value): bool } foreach ($value as $ringIndex => $ring) { - if (! is_array($ring) || empty($ring)) { + if (! is_array($ring) || empty($ring) || ! array_is_list($ring)) { $this->message = "Ring #{$ringIndex} must be an array of points"; return false; @@ -113,7 +113,7 @@ protected function validatePolygon(array $value): bool } foreach ($ring as $pointIndex => $point) { - if (! is_array($point) || count($point) !== 2) { + if (! is_array($point) || count($point) !== 2 || ! array_is_list($point)) { $this->message = "Point #{$pointIndex} in ring #{$ringIndex} must be an array of two values [x, y]"; return false; diff --git a/tests/e2e/Adapter/Scopes/RelationshipTests.php b/tests/e2e/Adapter/Scopes/RelationshipTests.php index 10337cf8c..ad30ee21d 100644 --- a/tests/e2e/Adapter/Scopes/RelationshipTests.php +++ b/tests/e2e/Adapter/Scopes/RelationshipTests.php @@ -3536,7 +3536,7 @@ public function testRelationshipSpatialQueries(): void [-73.99, 40.76], ]; $restaurants = $database->find('restaurantsSpatial', [ - Query::intersects('supplier.deliveryRoute', [$testLine]), + Query::intersects('supplier.deliveryRoute', $testLine), ]); // Verify query executes (result count depends on DB spatial implementation) $this->assertGreaterThanOrEqual(0, count($restaurants)); @@ -3547,7 +3547,7 @@ public function testRelationshipSpatialQueries(): void [-73.95, 40.80], ]; $restaurants = $database->find('restaurantsSpatial', [ - Query::crosses('supplier.deliveryRoute', [$crossingLine]), + Query::crosses('supplier.deliveryRoute', $crossingLine), ]); // Result depends on actual geometry intersection diff --git a/tests/e2e/Adapter/Scopes/SpatialTests.php b/tests/e2e/Adapter/Scopes/SpatialTests.php index 93f5c168e..e0fa228c4 100644 --- a/tests/e2e/Adapter/Scopes/SpatialTests.php +++ b/tests/e2e/Adapter/Scopes/SpatialTests.php @@ -1069,7 +1069,7 @@ public function testComplexGeometricShapes(): void // Test failure case: rectangle should NOT contain distant point if ($this->spatialIncludesBoundaries()) { $distantPoint = $database->find($collectionName, [ - Query::covers('rectangle', [[100, 100]]), // Point far outside rectangle + Query::covers('rectangle', [[100, 80]]), // Point far outside rectangle ], PermissionType::Read); $this->assertEmpty($distantPoint); } @@ -1171,7 +1171,7 @@ public function testComplexGeometricShapes(): void // Test failure case: square should NOT contain distant point if ($this->spatialIncludesBoundaries()) { $distantPointSquare = $database->find($collectionName, [ - Query::covers('square', [[100, 100]]), // Point far outside square + Query::covers('square', [[100, 80]]), // Point far outside square ], PermissionType::Read); $this->assertEmpty($distantPointSquare); } @@ -1223,7 +1223,7 @@ public function testComplexGeometricShapes(): void // Test failure case: triangle should NOT contain distant point if ($this->spatialIncludesBoundaries()) { $distantPointTriangle = $database->find($collectionName, [ - Query::covers('triangle', [[100, 100]]), // Point far outside triangle + Query::covers('triangle', [[100, 80]]), // Point far outside triangle ], PermissionType::Read); $this->assertEmpty($distantPointTriangle); } @@ -1268,7 +1268,7 @@ public function testComplexGeometricShapes(): void // Test failure case: L-shaped polygon should NOT contain distant point if ($this->spatialIncludesBoundaries()) { $distantPointLShape = $database->find($collectionName, [ - Query::covers('complex_polygon', [[100, 100]]), // Point far outside L-shape + Query::covers('complex_polygon', [[100, 80]]), // Point far outside L-shape ], PermissionType::Read); $this->assertEmpty($distantPointLShape); } @@ -1293,7 +1293,7 @@ public function testComplexGeometricShapes(): void // Test failure case: T-shaped polygon should NOT contain distant point if ($this->spatialIncludesBoundaries()) { $distantPointTShape = $database->find($collectionName, [ - Query::covers('complex_polygon', [[100, 100]]), // Point far outside T-shape + Query::covers('complex_polygon', [[100, 80]]), // Point far outside T-shape ], PermissionType::Read); $this->assertEmpty($distantPointTShape); } diff --git a/tests/unit/EngineErrorMappingTest.php b/tests/unit/EngineErrorMappingTest.php index 4f7f0144e..6ad0bf2cb 100644 --- a/tests/unit/EngineErrorMappingTest.php +++ b/tests/unit/EngineErrorMappingTest.php @@ -105,6 +105,33 @@ public function testLockConflictsMissingTablesAndBadCharactersAreMapped(Closure $this->assertMapped($map, $error, $expected, $message); } + /** + * @return iterable + */ + public static function invalidInputProvider(): iterable + { + $invalidGeometry = self::engineError('22023', 3037, 'SQLSTATE[22023]: Invalid parameter value: 3037 Invalid GIS data provided to function st_geomfromtext.'); + yield 'MariaDB invalid geometry' => [self::mariaDB(), $invalidGeometry, 'Invalid spatial value']; + + $geometries = [ + 'invalid geometry' => $invalidGeometry, + 'longitude out of range' => self::engineError('22S02', 3616, 'SQLSTATE[22S02]: <>: 3616 Longitude 999.000000 is out of range in function st_geomfromtext. It must be within (-180.000000, 180.000000].'), + 'latitude out of range' => self::engineError('22S03', 3617, 'SQLSTATE[22S03]: <>: 3617 Latitude 999.000000 is out of range in function st_geomfromtext. It must be within [-90.000000, 90.000000].'), + ]; + foreach ($geometries as $name => $error) { + yield "MySQL {$name}" => [self::mySQL(), $error, 'Invalid spatial value']; + } + } + + /** + * @param Closure(PDOException): Throwable $map + */ + #[DataProvider('invalidInputProvider')] + public function testInvalidInputTheEngineRefusesIsAQueryError(Closure $map, PDOException $error, string $message): void + { + $this->assertMapped($map, $error, QueryException::class, $message); + } + /** * @return array, 2: string}> */ diff --git a/tests/unit/Validator/Query/FilterGeometryTest.php b/tests/unit/Validator/Query/FilterGeometryTest.php new file mode 100644 index 000000000..2b0b4448f --- /dev/null +++ b/tests/unit/Validator/Query/FilterGeometryTest.php @@ -0,0 +1,113 @@ + 'pt', 'key' => 'pt', 'type' => ColumnType::Point->value, 'array' => false]), + new Document(['$id' => 'pg', 'key' => 'pg', 'type' => ColumnType::Polygon->value, 'array' => false]), + ], + idAttributeType: ColumnType::Integer->value, + ); + } + + /** + * @return iterable + */ + public static function malformed(): iterable + { + yield 'nested too deep' => [Query::intersects('pg', [[[[[1]]]]])]; + yield 'longitude and latitude out of range' => [Query::intersects('pt', [[999, 999]])]; + yield 'latitude out of range' => [Query::intersects('pt', [10, 91])]; + yield 'coordinates that are not numbers' => [Query::intersects('pt', [['a', 'b']])]; + yield 'a point of three coordinates' => [Query::intersects('pt', [1, 2, 3])]; + yield 'a point given as an object' => [Query::intersects('pt', [['x' => 1, 'y' => 2]])]; + yield 'a ring that is not closed' => [Query::intersects('pg', [[[0, 0], [0, 10], [10, 10], [10, 0]]])]; + yield 'a ring of too few points' => [Query::intersects('pg', [[[0, 0], [0, 10], [0, 0]]])]; + yield 'a linestring with a point of strings' => [Query::crosses('pg', [[0, 0], ['a', 'b']])]; + yield 'a distance from a point out of range' => [Query::distanceLessThan('pt', [999, 999], 10)]; + yield 'a distance from a point of strings' => [Query::distanceLessThan('pt', [['a', 'b']], 10)]; + yield 'a distance from text that is not WKT' => [new Query(Method::DistanceLessThan, 'pt', [['nowhere', 10, false]])]; + } + + #[DataProvider('malformed')] + public function testAMalformedGeometryIsRefused(Query $query): void + { + $this->assertFalse($this->validator()->isValid($query)); + } + + /** + * @return iterable + */ + public static function wellFormed(): iterable + { + yield 'a point' => [Query::intersects('pt', [1, 2])]; + yield 'a wrapped point' => [Query::intersects('pt', [[1, 2]])]; + yield 'a linestring' => [Query::crosses('pg', [[0, 0], [5, 5]])]; + yield 'a polygon' => [Query::intersects('pt', self::SQUARE)]; + yield 'a polygon on a polygon' => [Query::covers('pg', self::SQUARE)]; + yield 'equal to a stored polygon' => [Query::equal('pg', [self::SQUARE])]; + yield 'a distance from a point' => [Query::distanceLessThan('pt', [1, 2], 10, true)]; + yield 'a distance from WKT' => [new Query(Method::DistanceGreaterThan, 'pt', [['POINT(1 2)', 10, false]])]; + yield 'numeric strings' => [Query::intersects('pt', ['1.5', '2'])]; + } + + #[DataProvider('wellFormed')] + public function testAWellFormedGeometryIsValid(Query $query): void + { + $validator = $this->validator(); + + $this->assertTrue($validator->isValid($query), $validator->getDescription()); + } + + public function testTheRefusalNamesWhatIsWrong(): void + { + $validator = $this->validator(); + + $this->assertFalse($validator->isValid(Query::intersects('pt', [[999, 999]]))); + $this->assertSame( + 'Invalid spatial data for attribute "pt": Value must be a valid point: Longitude (x) must be between -180 and 180, got 999', + $validator->getDescription(), + ); + } + + /** + * @return iterable}> + */ + public static function storedObjects(): iterable + { + yield 'point' => [ColumnType::Point->value, ['x' => 1, 'y' => 2]]; + yield 'linestring point' => [ColumnType::Linestring->value, [[0, 0], ['x' => 1, 'y' => 2]]]; + yield 'polygon ring' => [ColumnType::Polygon->value, [['a' => [0, 0], 'b' => [0, 1], 'c' => [1, 1], 'd' => [0, 0]]]]; + yield 'polygon point' => [ColumnType::Polygon->value, [[[0, 0], ['x' => 0, 'y' => 1], [1, 1], [0, 0]]]]; + yield 'polygon single ring' => [ColumnType::Polygon->value, [['x' => 0, 'y' => 0], [0, 1], [1, 1], [0, 0]]]; + } + + /** + * @param array $value + */ + #[DataProvider('storedObjects')] + public function testAStoredCoordinateGivenAsAnObjectIsRefused(string $type, array $value): void + { + $this->assertFalse((new Spatial($type))->isValid($value)); + } +} From b052682c9eaa85c03f459fbdb77caa91fa40ef60 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 16 Oct 2026 15:00:18 +1300 Subject: [PATCH 2/2] fix(validator): parse spatial WKT and refuse coordinates that are not finite The Spatial validator took any string that began with POINT(, LINESTRING( or POLYGON( as WKT, so a written value such as `POINT(1 2` or `POINT(200 0)` reached the engine: MySQL raised error 3037, a server error, and MariaDB stored it and warned on the read back. A distance query from WKT was checked the same way, so its coordinates were never held to their range. A coordinate of INF or NAN passed the range check, which no comparison with NAN can fail. WKT is now parsed, without a regex, into the coordinates of its type and held to the rules of a value of that type: a point of two numbers, closed rings of at least four points, coordinates in longitude and latitude range. A written value must be WKT of the attribute's own type, and is refused as an invalid structure otherwise, so Appwrite answers 400 rather than 500. A distance query from WKT is validated as its geometry is. A coordinate that is not finite is refused. Co-Authored-By: Claude Opus 5.5 --- src/Database/Validator/Query/Filter.php | 6 +- src/Database/Validator/Spatial.php | 136 ++++++++++++++++++- tests/e2e/Adapter/Scopes/SpatialTests.php | 154 ++++++++++++++++++++++ tests/unit/Validator/SpatialTest.php | 67 ++++++++++ 4 files changed, 354 insertions(+), 9 deletions(-) diff --git a/src/Database/Validator/Query/Filter.php b/src/Database/Validator/Query/Filter.php index 490d04b29..157a33aaa 100644 --- a/src/Database/Validator/Query/Filter.php +++ b/src/Database/Validator/Query/Filter.php @@ -465,8 +465,10 @@ private function isValidDistance(array $values): bool [$geometry, $distance, $meters] = $tuple; - $validator = new Geometry(); - if (\is_string($geometry) ? ! Spatial::isWktString($geometry) : ! $validator->isValid($geometry)) { + $validator = \is_string($geometry) + ? new Spatial((Spatial::parseWkt($geometry)[0] ?? ColumnType::Point)->value) + : new Geometry(); + if (! $validator->isValid($geometry)) { $this->message = 'Distance query requires a geometry as coordinates or WKT: '.$validator->getDescription(); return false; diff --git a/src/Database/Validator/Spatial.php b/src/Database/Validator/Spatial.php index 7c854a8f3..d93e342e9 100644 --- a/src/Database/Validator/Spatial.php +++ b/src/Database/Validator/Spatial.php @@ -143,16 +143,120 @@ protected function validatePolygon(array $value): bool } /** - * Check if a value is a valid WKT (Well-Known Text) string. - * - * @param string $value The string to check - * @return bool + * Whether a value is well-formed WKT (Well-Known Text) of a point, linestring or polygon. Its coordinates are not + * checked; isValid() checks a stored value's. */ public static function isWktString(string $value): bool { - $value = trim($value); + return self::parseWkt($value) !== null; + } + + /** + * The type and coordinates WKT names, as a stored value of that type holds them: `[x, y]` for a point, a list of + * points for a linestring and a list of rings for a polygon. Null for anything that is not well-formed WKT of a + * point, linestring or polygon. + * + * @return array{ColumnType, array}|null + */ + public static function parseWkt(string $value): ?array + { + $value = \trim($value); + $open = \strpos($value, '('); + if ($open === false || ! \str_ends_with($value, ')')) { + return null; + } + + $type = match (\strtoupper(\trim(\substr($value, 0, $open)))) { + 'POINT' => ColumnType::Point, + 'LINESTRING' => ColumnType::Linestring, + 'POLYGON' => ColumnType::Polygon, + default => null, + }; + if ($type === null) { + return null; + } + + $body = \substr($value, $open + 1, -1); + $coordinates = match ($type) { + ColumnType::Point => self::parseWktPoint($body), + ColumnType::Linestring => self::parseWktPoints($body), + default => self::parseWktRings($body), + }; + + return $coordinates === null ? null : [$type, $coordinates]; + } + + /** + * @return list|null + */ + private static function parseWktPoint(string $text): ?array + { + $tokens = \array_values(\array_filter( + \explode(' ', \str_replace(["\t", "\n", "\r"], ' ', $text)), + static fn (string $token): bool => $token !== '', + )); + + if (\count($tokens) !== 2 || ! \is_numeric($tokens[0]) || ! \is_numeric($tokens[1])) { + return null; + } + + return [(float) $tokens[0], (float) $tokens[1]]; + } + + /** + * @return list>|null + */ + private static function parseWktPoints(string $text): ?array + { + $points = []; + foreach (\explode(',', $text) as $point) { + $parsed = self::parseWktPoint($point); + if ($parsed === null) { + return null; + } + $points[] = $parsed; + } - return (bool) preg_match('/^(POINT|LINESTRING|POLYGON)\s*\(/i', $value); + return $points; + } + + /** + * @return list>>|null + */ + private static function parseWktRings(string $text): ?array + { + $rings = []; + $rest = \trim($text); + while ($rest !== '') { + if (! \str_starts_with($rest, '(')) { + return null; + } + + $close = \strpos($rest, ')'); + if ($close === false) { + return null; + } + + $ring = self::parseWktPoints(\substr($rest, 1, $close - 1)); + if ($ring === null) { + return null; + } + $rings[] = $ring; + + $rest = \trim(\substr($rest, $close + 1)); + if ($rest === '') { + break; + } + if (! \str_starts_with($rest, ',')) { + return null; + } + $rest = \trim(\substr($rest, 1)); + if ($rest === '') { + return null; + } + } + + return $rings === [] ? null : $rings; } #[\Override] @@ -192,7 +296,19 @@ public function isValid(mixed $value): bool } if (is_string($value)) { - return self::isWktString($value); + $wkt = self::parseWkt($value); + if ($wkt === null) { + $this->message = 'WKT must be a well-formed POINT, LINESTRING or POLYGON'; + + return false; + } + + [$type, $value] = $wkt; + if ($type->value !== $this->spatialType) { + $this->message = 'WKT must be a '.\strtoupper($this->spatialType).', got a '.\strtoupper($type->value); + + return false; + } } if (is_array($value)) { @@ -221,6 +337,12 @@ public function isValid(mixed $value): bool private function isValidCoordinate(int|float $x, int|float $y): bool { + if (! \is_finite($x) || ! \is_finite($y)) { + $this->message = 'Coordinates must be finite numbers'; + + return false; + } + if ($x < -180 || $x > 180) { $this->message = "Longitude (x) must be between -180 and 180, got {$x}"; diff --git a/tests/e2e/Adapter/Scopes/SpatialTests.php b/tests/e2e/Adapter/Scopes/SpatialTests.php index e0fa228c4..183213cee 100644 --- a/tests/e2e/Adapter/Scopes/SpatialTests.php +++ b/tests/e2e/Adapter/Scopes/SpatialTests.php @@ -2216,6 +2216,160 @@ public function testInvalidSpatialTypes(): void } } + /** + * A spatial filter's coordinates were checked for shape only, so a longitude or latitude out of range, a + * non-finite coordinate or malformed WKT reached the engine, which MySQL raised as error 3037, 3616 or 3617, a + * server error. Each is refused as an invalid query on find() and count() before the engine sees it, and the + * edges of the ranges keep working. + */ + public function testSpatialFilterOutOfRangeOrMalformedIsAnInvalidQuery(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + if (! ($database->getAdapter()->hasFeature(Feature\Spatial::class))) { + $this->expectNotToPerformAssertions(); + + return; + } + + $collection = 'spatial_ranges_'.\substr(\uniqid(), -6); + $database->createCollection(Collection::create( + id: $collection, + attributes: [ + Attribute::point(key: 'pt'), + Attribute::lineString(key: 'ln'), + Attribute::polygon(key: 'poly'), + ], + permissions: [Permission::create(Role::any()), Permission::read(Role::any()), Permission::update(Role::any())], + )); + $database->createDocument($collection, new Document([ + '$id' => 'origin', + 'pt' => [0.0, 0.0], + 'ln' => [[0.0, 0.0], [1.0, 1.0]], + 'poly' => [[[0.0, 0.0], [0.0, 1.0], [1.0, 1.0], [0.0, 0.0]]], + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::any())], + ])); + + $filters = [ + 'point past both ranges' => Query::parse('{"method":"intersects","attribute":"pt","values":[[200,100]]}'), + 'longitude just past 180' => Query::parse('{"method":"intersects","attribute":"pt","values":[[180.0001,0]]}'), + 'longitude just past -180' => Query::parse('{"method":"intersects","attribute":"pt","values":[[-180.0001,0]]}'), + 'latitude just past 90' => Query::parse('{"method":"intersects","attribute":"pt","values":[[0,90.0001]]}'), + 'latitude just past -90' => Query::parse('{"method":"intersects","attribute":"pt","values":[[0,-90.0001]]}'), + 'point at the largest double' => Query::parse('{"method":"intersects","attribute":"pt","values":[[1e308,1e308]]}'), + 'infinite longitude' => Query::intersects('pt', [\INF, 0.0]), + 'undefined latitude' => Query::intersects('pt', [0.0, \NAN]), + 'polygon at latitude 91' => Query::parse('{"method":"intersects","attribute":"poly","values":[[[[0,0],[0,91],[1,91],[0,0]]]]}'), + 'line at longitude 181' => Query::parse('{"method":"crosses","attribute":"ln","values":[[[0,0],[181,0]]]}'), + 'equal point out of range' => Query::parse('{"method":"equal","attribute":"pt","values":[[200,100]]}'), + 'distance from a point out of range' => Query::parse('{"method":"distanceLessThan","attribute":"pt","values":[[[200,100],10,false]]}'), + 'distance from WKT out of range' => Query::parse('{"method":"distanceLessThan","attribute":"pt","values":[["POINT(200 100)",10,false]]}'), + 'distance from malformed WKT' => Query::parse('{"method":"distanceLessThan","attribute":"pt","values":[["POINT(1 2",10,false]]}'), + ]; + foreach ($filters as $case => $query) { + foreach (['find' => fn () => $database->find($collection, [$query]), 'count' => fn () => $database->count($collection, [$query])] as $read => $call) { + try { + $call(); + $this->fail("{$read}() refuses a filter with a {$case}"); + } catch (\Throwable $exception) { + $this->assertInstanceOf(QueryException::class, $exception, "{$read}() of a {$case}: ".$exception->getMessage()); + } + } + } + + $edges = [ + Query::intersects('pt', [180.0, 90.0]), + Query::intersects('pt', [-180.0, -90.0]), + Query::distanceLessThan('pt', [180.0, 0.0], 1), + Query::parse('{"method":"distanceLessThan","attribute":"pt","values":[["POINT(0.5 0.5)",1,false]]}'), + Query::intersects('poly', [[[0.0, 0.0], [0.0, 90.0], [180.0, 90.0], [0.0, 0.0]]]), + ]; + foreach ($edges as $query) { + $database->find($collection, [$query]); + $database->count($collection, [$query]); + } + $this->assertSame(['origin'], \array_map( + static fn (Document $document): string => $document->getId(), + $database->find($collection, [Query::parse('{"method":"distanceLessThan","attribute":"pt","values":[["POINT(0.5 0.5)",1,false]]}')]), + )); + + $database->deleteCollection($collection); + } + + /** + * A WKT value written to a spatial attribute was checked by its prefix only, so `POINT(1 2` or a point out of range + * reached the engine, which MySQL raised as error 3037, a server error. A written value of malformed WKT, WKT of + * another type, or coordinates out of range or not finite is refused as an invalid structure on create and update. + */ + public function testSpatialWriteOfMalformedOrOutOfRangeValueIsAnInvalidStructure(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + if (! ($database->getAdapter()->hasFeature(Feature\Spatial::class))) { + $this->expectNotToPerformAssertions(); + + return; + } + + $collection = 'spatial_ranges_'.\substr(\uniqid(), -6); + $database->createCollection(Collection::create( + id: $collection, + attributes: [ + Attribute::point(key: 'pt'), + Attribute::lineString(key: 'ln'), + Attribute::polygon(key: 'poly'), + ], + permissions: [Permission::create(Role::any()), Permission::read(Role::any()), Permission::update(Role::any())], + )); + $database->createDocument($collection, new Document([ + '$id' => 'origin', + 'pt' => [0.0, 0.0], + 'ln' => [[0.0, 0.0], [1.0, 1.0]], + 'poly' => [[[0.0, 0.0], [0.0, 1.0], [1.0, 1.0], [0.0, 0.0]]], + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::any())], + ])); + + $writes = [ + 'unclosed WKT point' => ['pt' => 'POINT(1 2'], + 'WKT point out of range' => ['pt' => 'POINT(200 0)'], + 'WKT point at an undefined latitude' => ['pt' => 'POINT(0 NAN)'], + 'WKT point of three coordinates' => ['pt' => 'POINT(1 2 3)'], + 'WKT of another type' => ['pt' => 'LINESTRING(0 0, 1 1)'], + 'WKT with trailing text' => ['pt' => 'POINT(1 2) junk'], + 'point past both ranges' => ['pt' => [200.0, 100.0]], + 'longitude just past 180' => ['pt' => [180.0001, 0.0]], + 'latitude just past -90' => ['pt' => [0.0, -90.0001]], + 'infinite longitude' => ['pt' => [\INF, 0.0]], + 'undefined latitude' => ['pt' => [0.0, \NAN]], + 'WKT line at longitude 181' => ['ln' => 'LINESTRING(0 0, 181 0)'], + 'WKT polygon at latitude 91' => ['poly' => 'POLYGON((0 0, 0 91, 1 91, 0 0))'], + 'WKT polygon missing its closing parenthesis' => ['poly' => 'POLYGON((0 0, 0 1, 1 1, 0 0)'], + 'WKT polygon ring that is not closed' => ['poly' => 'POLYGON((0 0, 0 1, 1 1, 1 0))'], + ]; + foreach ($writes as $case => $attributes) { + foreach (['create' => fn () => $database->createDocument($collection, new Document([...$attributes, '$permissions' => [Permission::read(Role::any())]])), 'update' => fn () => $database->updateDocument($collection, 'origin', new Document($attributes))] as $write => $call) { + try { + $call(); + $this->fail("{$write} refuses a {$case}"); + } catch (\Throwable $exception) { + $this->assertInstanceOf(StructureException::class, $exception, "{$write} of a {$case}: ".$exception->getMessage()); + } + } + } + + $written = $database->createDocument($collection, new Document([ + 'pt' => 'POINT(-180 -90)', + 'ln' => 'LINESTRING(-180 -90, 180 90)', + 'poly' => 'POLYGON((0 0, 0 10, 10 10, 0 0), (1 1, 1 2, 2 2, 1 1))', + '$permissions' => [Permission::read(Role::any())], + ])); + $read = $database->getDocument($collection, $written->getId()); + $this->assertEquals([-180.0, -90.0], $read->getAttribute('pt')); + $this->assertEquals([[-180.0, -90.0], [180.0, 90.0]], $read->getAttribute('ln')); + + $database->deleteCollection($collection); + } + public function testSpatialDistanceInMeter(): void { /** @var Database $database */ diff --git a/tests/unit/Validator/SpatialTest.php b/tests/unit/Validator/SpatialTest.php index 6478e3bc5..4d909bb7a 100644 --- a/tests/unit/Validator/SpatialTest.php +++ b/tests/unit/Validator/SpatialTest.php @@ -82,6 +82,73 @@ public function test_wkt_strings(): void $this->assertFalse(Spatial::isWktString('POINT1(1 2)')); } + /** + * @return iterable + */ + public static function malformedWkt(): iterable + { + yield 'unclosed point' => ['POINT(1 2']; + yield 'point of one coordinate' => ['POINT(1)']; + yield 'point of three coordinates' => ['POINT(1 2 3)']; + yield 'point of a word' => ['POINT(a 2)']; + yield 'empty point' => ['POINT EMPTY']; + yield 'empty parentheses' => ['POINT()']; + yield 'trailing text' => ['POINT(1 2) junk']; + yield 'nested point' => ['POINT((1 2))']; + yield 'linestring with an empty point' => ['LINESTRING(0 0,,1 1)']; + yield 'polygon without ring parentheses' => ['POLYGON(0 0,1 0,1 1,0 0)']; + yield 'polygon missing its closing parenthesis' => ['POLYGON((0 0,1 0,1 1,0 0)']; + yield 'polygon with a dangling comma' => ['POLYGON((0 0,1 0,1 1,0 0),)']; + yield 'polygon with rings not separated' => ['POLYGON((0 0,1 0,1 1,0 0)(0 0,1 0,1 1,0 0))']; + yield 'unknown type' => ['CIRCLE(0 0,1)']; + yield 'prefixed type' => ['POINT1(1 2)']; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('malformedWkt')] + public function test_malformed_wkt_is_refused(string $wkt): void + { + $this->assertFalse(Spatial::isWktString($wkt)); + $this->assertNull(Spatial::parseWkt($wkt)); + foreach ([ColumnType::Point, ColumnType::Linestring, ColumnType::Polygon] as $type) { + $this->assertFalse((new Spatial($type->value))->isValid($wkt), $type->value); + } + } + + public function test_wkt_is_parsed_into_the_coordinates_of_its_type(): void + { + $this->assertSame([ColumnType::Point, [1.0, -2.5]], Spatial::parseWkt(" point ( 1\t-2.5 ) ")); + $this->assertSame([ColumnType::Linestring, [[0.0, 0.0], [1.0, 1.0]]], Spatial::parseWkt('LINESTRING(0 0, 1 1)')); + $this->assertSame( + [ColumnType::Polygon, [[[0.0, 0.0], [1.0, 0.0], [1.0, 1.0], [0.0, 0.0]], [[0.1, 0.1], [0.2, 0.1], [0.2, 0.2], [0.1, 0.1]]]], + Spatial::parseWkt('POLYGON((0 0, 1 0, 1 1, 0 0), (0.1 0.1, 0.2 0.1, 0.2 0.2, 0.1 0.1))'), + ); + } + + public function test_wkt_is_held_to_the_rules_of_its_type(): void + { + $point = new Spatial(ColumnType::Point->value); + $this->assertTrue($point->isValid('POINT(180 -90)')); + $this->assertTrue($point->isValid('POINT(-180 90)')); + $this->assertFalse($point->isValid('POINT(180.0001 0)')); + $this->assertFalse($point->isValid('POINT(0 -90.0001)')); + $this->assertFalse($point->isValid('POINT(1e308 0)')); + $this->assertFalse($point->isValid('LINESTRING(0 0, 1 1)')); + $this->assertStringContainsString('WKT must be a POINT, got a LINESTRING', $point->getDescription()); + + $this->assertFalse((new Spatial(ColumnType::Linestring->value))->isValid('LINESTRING(0 0)')); + $this->assertFalse((new Spatial(ColumnType::Polygon->value))->isValid('POLYGON((0 0, 1 0, 1 1, 1 0))')); + $this->assertFalse((new Spatial(ColumnType::Polygon->value))->isValid('POLYGON((0 0, 1 0, 0 0))')); + } + + public function test_coordinates_that_are_not_finite_are_refused(): void + { + $point = new Spatial(ColumnType::Point->value); + foreach ([[\INF, 0], [0, -\INF], [\NAN, 0], [0, \NAN]] as $coordinates) { + $this->assertFalse($point->isValid($coordinates)); + $this->assertStringContainsString('finite', $point->getDescription()); + } + } + public function test_invalid_coordinate(): void { // Point with invalid longitude