diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 9622917..135ef38 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -2,7 +2,7 @@ includes: - phpstan-baseline.neon parameters: - level: 5 + level: 9 paths: - src - config diff --git a/src/Exceptions/KmlParserException.php b/src/Exceptions/KmlParserException.php index 7144f56..ca2ff32 100644 --- a/src/Exceptions/KmlParserException.php +++ b/src/Exceptions/KmlParserException.php @@ -9,6 +9,11 @@ public static function fileNotFound(string $path): self return new self("KML file not found: {$path}"); } + public static function failedToRead(string $path): self + { + return new self("Unable to read KML file: {$path}"); + } + public static function noDataLoaded(): self { return new self('No KML data loaded'); diff --git a/src/KmlParser.php b/src/KmlParser.php index 44b2c66..894244c 100755 --- a/src/KmlParser.php +++ b/src/KmlParser.php @@ -10,6 +10,14 @@ use PlinCode\KmlParser\Validators\KmlValidator; use SimpleXMLElement; +/** + * @phpstan-import-type Position from ParsesCoordinates + * @phpstan-import-type PolygonBoundaries from ParsesCoordinates + * + * @phpstan-type Placemark array + * @phpstan-type Geometry array + * @phpstan-type Style array + */ class KmlParser { use ParsesCoordinates; @@ -25,7 +33,12 @@ class KmlParser public function __construct() { - $this->namespace = $this->packageConfig('kml-parser.namespace', $this->namespace); + $namespace = $this->packageConfig('kml-parser.namespace', $this->namespace); + + if (is_string($namespace) && $namespace !== '') { + $this->namespace = $namespace; + } + $this->validator = new KmlValidator($this->supportedNamespaces()); } @@ -38,8 +51,17 @@ public function __construct() protected function supportedNamespaces(): array { $supported = $this->packageConfig('kml-parser.supported_namespaces', KmlValidator::DEFAULT_NAMESPACES); + $supported = is_array($supported) ? $supported : []; + + $namespaces = [$this->namespace]; - return array_values(array_unique(array_merge([$this->namespace], (array) $supported))); + foreach ($supported as $namespace) { + if (is_string($namespace) && $namespace !== '') { + $namespaces[] = $namespace; + } + } + + return array_values(array_unique($namespaces)); } /** @@ -53,7 +75,19 @@ public function loadFromFile(string $path): self throw KmlParserException::fileNotFound($path); } - return $this->loadFromString(file_get_contents($path)); + /* + * The warning file_get_contents() raises carries less than the + * exception below, and an application turning warnings into + * exceptions would otherwise get that one instead of ours. The return + * value is what is acted on. + */ + $content = @file_get_contents($path); + + if ($content === false) { + throw KmlParserException::failedToRead($path); + } + + return $this->loadFromString($content); } /** @@ -99,6 +133,8 @@ public function loadFromKmz(string $path): self /** * Get Placemarks Node from the KML * + * @return list + * * @throws Exception */ public function getPlacemarks(): array @@ -108,7 +144,7 @@ public function getPlacemarks(): array } $placemarks = []; - $placemarksXml = $this->xml->xpath('//kml:Placemark'); + $placemarksXml = $this->xml->xpath('//kml:Placemark') ?: []; foreach ($placemarksXml as $placemarkXml) { $placemark = [ @@ -225,6 +261,8 @@ protected function parseMultiGeometry(SimpleXMLElement $multiGeometry): array /** * Get Style Node from the KML * + * @return array + * * @throws Exception */ public function getStyles(): array @@ -234,7 +272,7 @@ public function getStyles(): array } $styles = []; - $stylesXml = $this->xml->xpath('//kml:Style'); + $stylesXml = $this->xml->xpath('//kml:Style') ?: []; foreach ($stylesXml as $styleXml) { $id = (string) $styleXml->attributes()->id; @@ -376,6 +414,8 @@ protected function parsePolyStyle(SimpleXMLElement $polyStyle): array /** * Get StyleMap Node from the KML * + * @return array}> + * * @throws Exception */ public function getStyleMaps(): array @@ -385,7 +425,7 @@ public function getStyleMaps(): array } $styleMaps = []; - $styleMapsXml = $this->xml->xpath('//kml:StyleMap'); + $styleMapsXml = $this->xml->xpath('//kml:StyleMap') ?: []; foreach ($styleMapsXml as $styleMapXml) { $id = (string) $styleMapXml->attributes()->id; @@ -410,6 +450,8 @@ public function getStyleMaps(): array /** * Convert data to GeoJSON format * + * @return array{type: string, features: list>} + * * @throws Exception */ public function toGeoJson(): array @@ -457,67 +499,129 @@ public function toGeoJson(): array * A KML MultiGeometry maps onto a GeoJSON GeometryCollection, which nests * the same way, so this recurses alongside parseMultiGeometry(). * - * @param array $geometry + * @param array $geometry * @return array|null */ protected function toGeoJsonGeometry(array $geometry): ?array { - return match ($geometry['type'] ?? null) { + $type = $geometry['type'] ?? null; + + if ($type === GeometryType::MULTI_GEOMETRY->value) { + return [ + 'type' => 'GeometryCollection', + 'geometries' => $this->toGeoJsonGeometries($geometry['geometries'] ?? []), + ]; + } + + $coordinates = $geometry['coordinates'] ?? null; + + if (! is_array($coordinates)) { + return null; + } + + return match ($type) { GeometryType::POINT->value => [ 'type' => 'Point', - 'coordinates' => $this->toGeoJsonPosition($geometry['coordinates']), + 'coordinates' => $this->toGeoJsonPosition($coordinates), ], GeometryType::LINE_STRING->value => [ 'type' => 'LineString', - 'coordinates' => array_map( - fn (array $position) => $this->toGeoJsonPosition($position), - $geometry['coordinates'], - ), + 'coordinates' => $this->toGeoJsonPositions($coordinates), ], GeometryType::POLYGON->value => [ 'type' => 'Polygon', - 'coordinates' => $this->toGeoJsonRings($geometry['coordinates']), - ], - GeometryType::MULTI_GEOMETRY->value => [ - 'type' => 'GeometryCollection', - 'geometries' => array_values(array_filter(array_map( - fn (array $child) => $this->toGeoJsonGeometry($child), - $geometry['geometries'], - ))), + 'coordinates' => $this->toGeoJsonRings($coordinates), ], default => null, }; } /** - * @param array{longitude: float, latitude: float, altitude: float} $position - * @return array + * @return list> + */ + protected function toGeoJsonGeometries(mixed $geometries): array + { + if (! is_array($geometries)) { + return []; + } + + $converted = []; + + foreach ($geometries as $child) { + if (! is_array($child)) { + continue; + } + + $geometry = $this->toGeoJsonGeometry($child); + + if ($geometry !== null) { + $converted[] = $geometry; + } + } + + return $converted; + } + + /** + * @param array $positions + * @return list> + */ + protected function toGeoJsonPositions(array $positions): array + { + $converted = []; + + foreach ($positions as $position) { + if (is_array($position)) { + $converted[] = $this->toGeoJsonPosition($position); + } + } + + return $converted; + } + + /** + * A coordinate map as parsePointCoordinates() produces it, turned into the + * GeoJSON position order. Anything not numeric reads as 0.0 rather than + * throwing, so one malformed coordinate cannot take a whole document down. + * + * @param array $position + * @return list */ protected function toGeoJsonPosition(array $position): array { - return [$position['longitude'], $position['latitude'], $position['altitude']]; + return [ + $this->toFloat($position['longitude'] ?? null), + $this->toFloat($position['latitude'] ?? null), + $this->toFloat($position['altitude'] ?? null), + ]; + } + + protected function toFloat(mixed $value): float + { + return is_numeric($value) ? (float) $value : 0.0; } /** * GeoJSON puts the outer ring first and every inner ring after it. * - * @param array{outerBoundary: array>, innerBoundaries: array>>} $boundaries - * @return array>> + * @param array $boundaries + * @return list>> */ protected function toGeoJsonRings(array $boundaries): array { - $rings = [ - array_map( - fn (array $position) => $this->toGeoJsonPosition($position), - $boundaries['outerBoundary'], - ), - ]; + $outer = $boundaries['outerBoundary'] ?? []; + $inner = $boundaries['innerBoundaries'] ?? []; + + $rings = [is_array($outer) ? $this->toGeoJsonPositions($outer) : []]; - foreach ($boundaries['innerBoundaries'] as $innerBoundary) { - $rings[] = array_map( - fn (array $position) => $this->toGeoJsonPosition($position), - $innerBoundary, - ); + if (! is_array($inner)) { + return $rings; + } + + foreach ($inner as $innerBoundary) { + if (is_array($innerBoundary)) { + $rings[] = $this->toGeoJsonPositions($innerBoundary); + } } return $rings; diff --git a/src/KmzExtractor.php b/src/KmzExtractor.php index 6431ebc..be96529 100644 --- a/src/KmzExtractor.php +++ b/src/KmzExtractor.php @@ -128,8 +128,8 @@ protected function open(string $path): ZipArchive */ protected function guardArchive(ZipArchive $zip): void { - $maxEntries = (int) $this->packageConfig('kml-parser.max_archive_entries', self::DEFAULT_MAX_ENTRIES); - $maxSize = (int) $this->packageConfig('kml-parser.max_uncompressed_size', self::DEFAULT_MAX_UNCOMPRESSED_SIZE); + $maxEntries = $this->configuredLimit('kml-parser.max_archive_entries', self::DEFAULT_MAX_ENTRIES); + $maxSize = $this->configuredLimit('kml-parser.max_uncompressed_size', self::DEFAULT_MAX_UNCOMPRESSED_SIZE); if ($maxEntries > 0 && $zip->numFiles > $maxEntries) { throw KmzExtractorException::tooManyEntries($zip->numFiles, $maxEntries); @@ -154,6 +154,18 @@ protected function guardArchive(ZipArchive $zip): void } } + /** + * A limit that is not a number is a misconfiguration, and silently reading + * it as 0 would turn the limit off, which is the opposite of what someone + * setting it wants. The documented default is used instead. + */ + protected function configuredLimit(string $key, int $default): int + { + $value = $this->packageConfig($key, $default); + + return is_numeric($value) ? (int) $value : $default; + } + /** * @throws KmzExtractorException */ diff --git a/src/Traits/ParsesCoordinates.php b/src/Traits/ParsesCoordinates.php index b612246..ce00d14 100644 --- a/src/Traits/ParsesCoordinates.php +++ b/src/Traits/ParsesCoordinates.php @@ -4,10 +4,14 @@ use SimpleXMLElement; +/** + * @phpstan-type Position array{longitude: float, latitude: float, altitude: float} + * @phpstan-type PolygonBoundaries array{outerBoundary: list, innerBoundaries: list>} + */ trait ParsesCoordinates { /** - * @return array{longitude: float, latitude: float, altitude: float} + * @return Position */ protected function parsePointCoordinates(string $coordinates): array { @@ -20,10 +24,13 @@ protected function parsePointCoordinates(string $coordinates): array ]; } + /** + * @return list + */ protected function parseLineStringCoordinates(string $coordinates): array { $coords = []; - $points = preg_split('/\s+/', trim($coordinates)); + $points = preg_split('/\s+/', trim($coordinates)) ?: []; foreach ($points as $point) { if (empty(trim($point))) { @@ -43,6 +50,9 @@ protected function parseLineStringCoordinates(string $coordinates): array return $coords; } + /** + * @return PolygonBoundaries + */ protected function parsePolygonCoordinates(SimpleXMLElement $polygon): array { $result = [ diff --git a/src/Validators/KmlValidator.php b/src/Validators/KmlValidator.php index 059a91a..f759591 100644 --- a/src/Validators/KmlValidator.php +++ b/src/Validators/KmlValidator.php @@ -167,7 +167,7 @@ protected function validateGeometryCoordinates(SimpleXMLElement $geometry, strin throw new KmlException('Empty coordinates in geometry'); } - $coords = preg_split('/\s+/', trim($coordinates)); + $coords = preg_split('/\s+/', trim($coordinates)) ?: []; foreach ($coords as $coord) { if (empty(trim($coord))) { continue; diff --git a/tests/KmzHardeningTest.php b/tests/KmzHardeningTest.php index cb011f6..e9a8a59 100644 --- a/tests/KmzHardeningTest.php +++ b/tests/KmzHardeningTest.php @@ -145,3 +145,13 @@ function removeRecursively(string $path): void expect(fn () => (new KmzExtractor)->extractAllFiles($path, $blocker.'/inside')) ->toThrow(KmzExtractorException::class, 'Unable to create the extraction directory'); }); + +it('falls back to the documented default when a limit is not a number', function () { + config()->set('kml-parser.max_archive_entries', 'plenty'); + + $path = makeArchive(function (ZipArchive $zip) { + $zip->addFromString('doc.kml', validKml()); + }); + + expect((new KmzExtractor)->extractKmlContent($path))->toContain(''); + chmod($path, 0000); + + try { + expect(fn () => (new KmlParser)->loadFromFile($path)) + ->toThrow(KmlParserException::class, 'Unable to read KML file'); + } finally { + chmod($path, 0644); + unlink($path); + } +})->skipOnWindows(); + +it('still reports a file that is not there at all', function () { + expect(fn () => (new KmlParser)->loadFromFile('/no/such/file.kml')) + ->toThrow(KmlParserException::class, 'KML file not found'); +});