From 287160a8c7578ae0f7e0e66eb83ff1f64ffc33cc Mon Sep 17 00:00:00 2001 From: Daniele Barbaro Date: Tue, 8 Sep 2026 14:41:57 +0200 Subject: [PATCH 1/2] fix(parser): stop swallowing validation errors loadFromString() wrapped its whole body in `catch (\Exception)` and rethrew everything as KmlParserException::failedToParse(). A validation failure raised by KmlValidator therefore reached the caller as a generic parse error, so "Invalid longitude value: 181" and "malformed XML" were indistinguishable by type. Validation exceptions now propagate untouched and malformed XML surfaces as invalidXml(), which until now was unreachable because the catch block below it re-wrapped it immediately. The content was also parsed twice, once by the validator and once by the parser, doubling the work and the peak memory of every load. The validator gained validateDocument(), which takes an already parsed document, and the parser builds the SimpleXMLElement once and hands it over. validate() keeps its string signature and delegates. Both classes flipped libxml_use_internal_errors(true) on and never restored it, changing libxml error handling for the rest of the application. The previous value is now saved and restored in a finally, on the success and failure paths alike. failedToParse() is no longer thrown and is marked deprecated rather than removed, so callers referencing it keep working for one cycle. --- src/Exceptions/KmlParserException.php | 7 +++ src/KmlParser.php | 28 +++++----- src/Validators/KmlValidator.php | 60 ++++++++++++++-------- tests/ExceptionsTest.php | 2 +- tests/ParsingErrorsTest.php | 74 +++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 38 deletions(-) create mode 100644 tests/ParsingErrorsTest.php diff --git a/src/Exceptions/KmlParserException.php b/src/Exceptions/KmlParserException.php index 392d398..7144f56 100644 --- a/src/Exceptions/KmlParserException.php +++ b/src/Exceptions/KmlParserException.php @@ -19,6 +19,13 @@ public static function invalidXml(string $message): self return new self("XML parsing error: {$message}"); } + /** + * @deprecated Nothing throws this any more. Malformed XML now surfaces as + * invalidXml(), and a validation failure keeps its own + * KmlException instead of being wrapped. Kept for one cycle so + * callers referencing it do not break; slated for removal in + * the next major. + */ public static function failedToParse(string $message): self { return new self("Failed to parse KML content: {$message}"); diff --git a/src/KmlParser.php b/src/KmlParser.php index ceb2336..5a6befa 100755 --- a/src/KmlParser.php +++ b/src/KmlParser.php @@ -46,25 +46,23 @@ public function loadFromFile(string $path): self */ public function loadFromString(string $content): self { - try { - $this->validator->validate($content); - libxml_use_internal_errors(true); - $this->xml = new SimpleXMLElement($content); - - $errors = libxml_get_errors(); - if ($errors) { - $errorMessage = $errors[0]->message; - libxml_clear_errors(); - throw KmlParserException::invalidXml($errorMessage); - } + $previous = libxml_use_internal_errors(true); - $this->xml->registerXPathNamespace('kml', $this->namespace); - - return $this; + try { + $xml = new SimpleXMLElement($content); } catch (Exception $e) { + throw KmlParserException::invalidXml($e->getMessage()); + } finally { libxml_clear_errors(); - throw KmlParserException::failedToParse($e->getMessage()); + libxml_use_internal_errors($previous); } + + $this->validator->validateDocument($xml); + + $this->xml = $xml; + $this->xml->registerXPathNamespace('kml', $this->namespace); + + return $this; } /** diff --git a/src/Validators/KmlValidator.php b/src/Validators/KmlValidator.php index f814b13..50cd058 100644 --- a/src/Validators/KmlValidator.php +++ b/src/Validators/KmlValidator.php @@ -12,36 +12,52 @@ class KmlValidator protected SimpleXMLElement $xml; + /** + * Parse and validate raw KML content. + * + * @throws KmlException + */ public function validate(string $content): void { - libxml_use_internal_errors(true); + $previous = libxml_use_internal_errors(true); try { - $this->xml = new SimpleXMLElement($content); - - $namespaces = $this->xml->getDocNamespaces(); - if (! isset($namespaces['']) || $namespaces[''] !== $this->namespace) { - throw new KmlException('Invalid or missing KML namespace'); - } - - $this->xml->registerXPathNamespace('kml', $this->namespace); - - if (empty($this->xml->Document)) { - throw new KmlException('Missing required element: Document'); - } - - $placemarks = $this->xml->xpath('//kml:Placemark'); - if (! empty($placemarks)) { - foreach ($placemarks as $placemark) { - $this->validatePlacemark($placemark); - } - } - } catch (KmlException $e) { - throw $e; + $xml = new SimpleXMLElement($content); } catch (\Exception $e) { throw new KmlException('Invalid KML content: '.$e->getMessage()); } finally { libxml_clear_errors(); + libxml_use_internal_errors($previous); + } + + $this->validateDocument($xml); + } + + /** + * Validate an already parsed KML document. + * + * Callers that have parsed the document themselves should use this instead + * of validate(), so the content is not parsed twice. + * + * @throws KmlException + */ + public function validateDocument(SimpleXMLElement $xml): void + { + $this->xml = $xml; + + $namespaces = $xml->getDocNamespaces(); + if (! isset($namespaces['']) || $namespaces[''] !== $this->namespace) { + throw new KmlException('Invalid or missing KML namespace'); + } + + $xml->registerXPathNamespace('kml', $this->namespace); + + if (empty($xml->Document)) { + throw new KmlException('Missing required element: Document'); + } + + foreach ($xml->xpath('//kml:Placemark') ?: [] as $placemark) { + $this->validatePlacemark($placemark); } } diff --git a/tests/ExceptionsTest.php b/tests/ExceptionsTest.php index ca8a36c..d92a25e 100644 --- a/tests/ExceptionsTest.php +++ b/tests/ExceptionsTest.php @@ -21,7 +21,7 @@ $parser = new KmlParser; $parser->loadFromString('invalid xml content'); -})->throws(KmlParserException::class, 'Failed to parse KML content'); +})->throws(KmlParserException::class, 'XML parsing error'); it('throws exception when KMZ file not found', function () { $extractor = new KmzExtractor; diff --git a/tests/ParsingErrorsTest.php b/tests/ParsingErrorsTest.php new file mode 100644 index 0000000..c28394f --- /dev/null +++ b/tests/ParsingErrorsTest.php @@ -0,0 +1,74 @@ + + + + + + 7.7300965,{$latitude},0 + + + + +XML; +} + +it('surfaces a validation failure with its own type and message', function () { + try { + (new KmlParser)->loadFromString(kmlWithLatitude('91')); + } catch (KmlException $e) { + expect($e)->not->toBeInstanceOf(KmlParserException::class) + ->and($e->getMessage())->toBe('Invalid latitude value: 91'); + + return; + } + + $this->fail('No exception was thrown.'); +}); + +it('reports malformed XML as a parsing error', function () { + try { + (new KmlParser)->loadFromString(''); + } catch (KmlParserException $e) { + expect($e->getMessage())->toStartWith('XML parsing error: '); + + return; + } + + $this->fail('No exception was thrown.'); +}); + +it('restores the libxml error handling mode after a successful load', function () { + $before = libxml_use_internal_errors(false); + + (new KmlParser)->loadFromString(kmlWithLatitude('45.8635629')); + + expect(libxml_use_internal_errors($before))->toBeFalse(); +}); + +it('restores the libxml error handling mode after a failed load', function () { + $before = libxml_use_internal_errors(false); + + try { + (new KmlParser)->loadFromString(''); + } catch (KmlParserException) { + // The state has to be restored on the failure path too. + } + + expect(libxml_use_internal_errors($before))->toBeFalse(); +}); + +it('validates an already parsed document without reparsing it', function () { + $xml = new SimpleXMLElement(kmlWithLatitude('91')); + + expect(fn () => (new KmlValidator)->validateDocument($xml)) + ->toThrow(KmlException::class, 'Invalid latitude value: 91'); +}); From 2a454b7c0451a3b449eda48d86a782f3ce4e1ba8 Mon Sep 17 00:00:00 2001 From: Daniele Barbaro Date: Tue, 8 Sep 2026 14:46:10 +0200 Subject: [PATCH 2/2] feat(validator): accept the legacy KML namespaces KmlValidator hardcoded the OGC 2.2 namespace and rejected everything else, so any document exported before the OGC took the format over (earth.google.com/kml/2.0, 2.1 and 2.2, still common in the wild) failed validation with "Invalid or missing KML namespace" and could not be parsed at all. The accepted namespaces are now a list, configurable through the new supported_namespaces key, and XPath is registered against the namespace the document actually declares rather than the one we assumed. That last part is what makes a 2.1 document parse end to end instead of validating and then returning no placemarks. The validator also ignored the kml-parser.namespace config entirely, even though KmlParser read it. Setting that key made every document invalid, since validation still demanded 2.2. KmlParser now passes the configured namespaces to the validator, so the key finally does what it says. KmlValidator keeps working standalone, defaulting to the known namespaces without touching the container. --- README.md | 20 +++++++++- config/kml-parser.php | 18 +++++++++ src/KmlParser.php | 17 +++++++- src/Validators/KmlValidator.php | 50 +++++++++++++++++++++-- tests/NamespaceSupportTest.php | 71 +++++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 tests/NamespaceSupportTest.php diff --git a/README.md b/README.md index 3948c33..92c1bc5 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,25 @@ return [ | */ 'namespace' => 'http://www.opengis.net/kml/2.2', - + + /* + |-------------------------------------------------------------------------- + | Accepted KML Namespaces + |-------------------------------------------------------------------------- + | + | A document is rejected unless it declares one of these as its default + | namespace. 2.2 is the OGC standard; the earth.google.com variants come + | from Google Earth and older exporters and are still common in the wild. + | XPath always runs against whichever one the document actually declares. + | + */ + 'supported_namespaces' => [ + 'http://www.opengis.net/kml/2.2', + 'http://earth.google.com/kml/2.2', + 'http://earth.google.com/kml/2.1', + 'http://earth.google.com/kml/2.0', + ], + /* |-------------------------------------------------------------------------- | Temporary Directory diff --git a/config/kml-parser.php b/config/kml-parser.php index 4833743..8ca3448 100644 --- a/config/kml-parser.php +++ b/config/kml-parser.php @@ -13,6 +13,24 @@ */ 'namespace' => 'http://www.opengis.net/kml/2.2', + /* + |-------------------------------------------------------------------------- + | Accepted KML Namespaces + |-------------------------------------------------------------------------- + | + | A document is rejected unless it declares one of these as its default + | namespace. 2.2 is the OGC standard; the earth.google.com variants come + | from Google Earth and older exporters and are still common in the wild. + | XPath always runs against whichever one the document actually declares. + | + */ + 'supported_namespaces' => [ + 'http://www.opengis.net/kml/2.2', + 'http://earth.google.com/kml/2.2', + 'http://earth.google.com/kml/2.1', + 'http://earth.google.com/kml/2.0', + ], + /* |-------------------------------------------------------------------------- | Temporary Directory diff --git a/src/KmlParser.php b/src/KmlParser.php index 5a6befa..82aaa41 100755 --- a/src/KmlParser.php +++ b/src/KmlParser.php @@ -22,7 +22,20 @@ class KmlParser public function __construct() { $this->namespace = config('kml-parser.namespace', $this->namespace); - $this->validator = new KmlValidator; + $this->validator = new KmlValidator($this->supportedNamespaces()); + } + + /** + * Namespaces a document is allowed to declare: the configured primary one + * plus every variant listed in the config. + * + * @return array + */ + protected function supportedNamespaces(): array + { + $supported = config('kml-parser.supported_namespaces', KmlValidator::DEFAULT_NAMESPACES); + + return array_values(array_unique(array_merge([$this->namespace], (array) $supported))); } /** @@ -60,7 +73,7 @@ public function loadFromString(string $content): self $this->validator->validateDocument($xml); $this->xml = $xml; - $this->xml->registerXPathNamespace('kml', $this->namespace); + $this->xml->registerXPathNamespace('kml', $this->validator->documentNamespace()); return $this; } diff --git a/src/Validators/KmlValidator.php b/src/Validators/KmlValidator.php index 50cd058..059a91a 100644 --- a/src/Validators/KmlValidator.php +++ b/src/Validators/KmlValidator.php @@ -8,10 +8,47 @@ class KmlValidator { - protected string $namespace = 'http://www.opengis.net/kml/2.2'; + /** + * The namespaces a KML document is allowed to declare. + * + * 2.2 is the OGC standard. The earth.google.com variants predate the OGC + * taking the format over, and exports carrying them are still in wide + * circulation, so rejecting them outright rejects valid files. + * + * @var array + */ + public const DEFAULT_NAMESPACES = [ + 'http://www.opengis.net/kml/2.2', + 'http://earth.google.com/kml/2.2', + 'http://earth.google.com/kml/2.1', + 'http://earth.google.com/kml/2.0', + ]; + + /** @var array */ + protected array $namespaces; + + protected string $documentNamespace = ''; protected SimpleXMLElement $xml; + /** + * @param array|null $namespaces Accepted namespaces, defaults to DEFAULT_NAMESPACES. + */ + public function __construct(?array $namespaces = null) + { + $namespaces = array_values(array_filter($namespaces ?? self::DEFAULT_NAMESPACES)); + + $this->namespaces = $namespaces !== [] ? $namespaces : self::DEFAULT_NAMESPACES; + } + + /** + * The namespace declared by the document that was validated last. + */ + public function documentNamespace(): string + { + return $this->documentNamespace; + } + /** * Parse and validate raw KML content. * @@ -46,11 +83,18 @@ public function validateDocument(SimpleXMLElement $xml): void $this->xml = $xml; $namespaces = $xml->getDocNamespaces(); - if (! isset($namespaces['']) || $namespaces[''] !== $this->namespace) { + $declared = $namespaces[''] ?? null; + + if ($declared === null || ! in_array($declared, $this->namespaces, true)) { throw new KmlException('Invalid or missing KML namespace'); } - $xml->registerXPathNamespace('kml', $this->namespace); + /* + * XPath has to run against the namespace the document actually + * declares, not the one we would have preferred it to use. + */ + $this->documentNamespace = $declared; + $xml->registerXPathNamespace('kml', $declared); if (empty($xml->Document)) { throw new KmlException('Missing required element: Document'); diff --git a/tests/NamespaceSupportTest.php b/tests/NamespaceSupportTest.php new file mode 100644 index 0000000..c5018cb --- /dev/null +++ b/tests/NamespaceSupportTest.php @@ -0,0 +1,71 @@ + + + + Legacy export + + Lago Blu + + 7.7300965,45.8635629,0 + + + + +XML; +} + +it('parses a document in the OGC 2.2 namespace', function () { + $parser = (new KmlParser)->loadFromString(kmlIn('http://www.opengis.net/kml/2.2')); + + expect($parser->getPlacemarks())->toHaveCount(1) + ->and($parser->getDocumentName())->toBe('Legacy export'); +}); + +it('parses documents in the legacy Google namespaces', function (string $namespace) { + $parser = (new KmlParser)->loadFromString(kmlIn($namespace)); + + expect($parser->getPlacemarks())->toHaveCount(1) + ->and($parser->getPlacemarks()[0]['name'])->toBe('Lago Blu') + ->and($parser->getDocumentName())->toBe('Legacy export'); +})->with([ + 'http://earth.google.com/kml/2.2', + 'http://earth.google.com/kml/2.1', + 'http://earth.google.com/kml/2.0', +]); + +it('still rejects a namespace that is not KML', function () { + expect(fn () => (new KmlParser)->loadFromString(kmlIn('http://wrong.namespace'))) + ->toThrow(KmlException::class, 'Invalid or missing KML namespace'); +}); + +it('accepts a namespace added through the config', function () { + config()->set('kml-parser.supported_namespaces', ['http://example.test/kml']); + + $parser = (new KmlParser)->loadFromString(kmlIn('http://example.test/kml')); + + expect($parser->getPlacemarks())->toHaveCount(1); +}); + +it('keeps accepting the configured primary namespace', function () { + config()->set('kml-parser.namespace', 'http://example.test/kml'); + config()->set('kml-parser.supported_namespaces', []); + + $parser = (new KmlParser)->loadFromString(kmlIn('http://example.test/kml')); + + expect($parser->getPlacemarks())->toHaveCount(1); +}); + +it('exposes the namespace the document declared', function () { + $validator = new KmlValidator; + $validator->validateDocument(new SimpleXMLElement(kmlIn('http://earth.google.com/kml/2.1'))); + + expect($validator->documentNamespace())->toBe('http://earth.google.com/kml/2.1'); +});