Bugfix: use libxml_get_errors instead of looping over libxml_get_last_error

Author: tvdijen

composer.json

@@ -32,7 +32,6 @@
         "ext-libxml": "*",
         "ext-pcre": "*",
         "ext-spl": "*",
-        "ext-xmlreader": "*",
 
         "simplesamlphp/assert": "^1.2",
         "simplesamlphp/composer-xmlprovider-installer": "~1.0.0",

src/DOMDocumentFactory.php

@@ -5,21 +5,19 @@
 namespace SimpleSAML\XML;
 
 use DOMDocument;
-use Exception;
-use LibXMLError;
 use SimpleSAML\Assert\Assert;
 use SimpleSAML\XML\Exception\IOException;
 use SimpleSAML\XML\Exception\RuntimeException;
 use SimpleSAML\XML\Exception\SchemaViolationException;
 use SimpleSAML\XML\Exception\UnparseableXMLException;
-use XMLReader;
 
 use function array_unique;
+use function file_exists;
 use function file_get_contents;
 use function func_num_args;
 use function implode;
 use function libxml_clear_errors;
-use function libxml_get_last_error;
+use function libxml_get_errors;
 use function libxml_set_external_entity_loader;
 use function libxml_use_internal_errors;
 use function sprintf;
@@ -150,29 +148,19 @@ public static function schemaValidation(
         string $schemaFile,
         int $options = self::DEFAULT_OPTIONS,
     ): void {
-        $xmlReader = XMLReader::XML($xml, null, $options);
-        Assert::notFalse($xmlReader, SchemaViolationException::class);
-
-        libxml_use_internal_errors(true);
-
-        try {
-            $xmlReader->setSchema($schemaFile);
-        } catch (Exception) {
-            $err = libxml_get_last_error();
-            throw new SchemaViolationException(trim($err->message) . ' on line ' . $err->line);
+        if (!file_exists($schemaFile)) {
+            throw new IOException('File not found.');
         }
 
-        $msgs = [];
-        while ($xmlReader->read()) {
-            if (!$xmlReader->isValid()) {
-                $err = libxml_get_last_error();
-                if ($err instanceof LibXMLError) {
-                    $msgs[] = trim($err->message) . ' on line ' . $err->line;
-                }
+        $document = DOMDocumentFactory::fromString($xml);
+        $result = $document->schemaValidate($schemaFile);
+
+        if ($result === false) {
+            $msgs = [];
+            foreach (libxml_get_errors() as $err) {
+                $msgs[] = trim($err->message) . ' on line ' . $err->line;
             }
-        }
 
-        if ($msgs) {
             throw new SchemaViolationException(sprintf(
                 "XML schema validation errors:\n - %s",
                 implode("\n - ", array_unique($msgs)),

tests/XML/DOMDocumentFactoryTest.php

@@ -5,7 +5,6 @@
 namespace SimpleSAML\Test\XML;
 
 use DOMDocument;
-use Exception;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\TestCase;
@@ -15,8 +14,6 @@
 use SimpleSAML\XML\Exception\SchemaViolationException;
 use SimpleSAML\XML\Exception\UnparseableXMLException;
 
-use function restore_error_handler;
-use function set_error_handler;
 use function strval;
 
 /**
@@ -145,13 +142,17 @@ public function testSchemaValidationUnknownSchemaFileFails(): void
         $file = 'tests/resources/xml/ssp_Chunk.xml';
         $schemaFile = 'tests/resources/schemas/doesnotexist.xsd';
 
-        // Dirty trick to catch the warning emitted by PHP
-        set_error_handler(static function (int $errno, string $errstr): never {
-            restore_error_handler();
-            throw new Exception($errstr, $errno);
-        }, E_WARNING);
+        $this->expectExceptionMessage('File not found.');
+        DOMDocumentFactory::fromFile($file, $schemaFile);
+    }
 
-        $this->expectExceptionMessage('Failed to locate the main schema resource at');
+
+    public function testSchemaValidationInvalidSchemaFileFails(): void
+    {
+        $file = 'tests/resources/xml/ssp_Chunk.xml';
+        $schemaFile = 'resources/schemas/xml.xsd';
+
+        $this->expectException(SchemaViolationException::class);
         DOMDocumentFactory::fromFile($file, $schemaFile);
     }
 }