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.

Author: danielebarbaro

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}");

src/KmlParser.php

@@ -45,25 +45,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;
-        } catch (\Exception $e) {
+        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;
     }
 
     /**

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);
         }
     }
 

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;

tests/ParsingErrorsTest.php

@@ -0,0 +1,74 @@
+<?php
+
+use PlinCode\KmlParser\Exceptions\KmlException;
+use PlinCode\KmlParser\Exceptions\KmlParserException;
+use PlinCode\KmlParser\KmlParser;
+use PlinCode\KmlParser\Validators\KmlValidator;
+
+function kmlWithLatitude(string $latitude): string
+{
+    return <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<kml xmlns="http://www.opengis.net/kml/2.2">
+    <Document>
+        <Placemark>
+            <Point>
+                <coordinates>7.7300965,{$latitude},0</coordinates>
+            </Point>
+        </Placemark>
+    </Document>
+</kml>
+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('<kml><unclosed>');
+    } 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('<kml><unclosed>');
+    } 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');
+});