Refactor schema validation

simplesamlphp · Dec 16, 2024 · e67ade3 · e67ade3
1 parent 221b312
commit e67ade3
Show file tree

Hide file tree

Showing 13 changed files with 120 additions and 120 deletions.
diff --git a/src/DOMDocumentFactory.php b/src/DOMDocumentFactory.php
@@ -8,20 +8,14 @@
 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 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_errors;
 use function libxml_set_external_entity_loader;
 use function libxml_use_internal_errors;
 use function sprintf;
-use function trim;
 
 /**
  * @package simplesamlphp/xml-common
@@ -37,14 +31,12 @@ final class DOMDocumentFactory
 
     /**
      * @param string $xml
-     * @param string|null $schemaFile
      * @param non-negative-int $options
      *
      * @return \DOMDocument
      */
     public static function fromString(
         string $xml,
-        ?string $schemaFile = null,
         int $options = self::DEFAULT_OPTIONS,
     ): DOMDocument {
         libxml_set_external_entity_loader(null);
@@ -64,11 +56,6 @@ public static function fromString(
             $options |= LIBXML_NO_XXE;
         }
 
-        // Perform optional schema validation
-        if (!empty($schemaFile)) {
-            self::schemaValidation($xml, $schemaFile, $options);
-        }
-
         $domDocument = self::create();
         $loaded = $domDocument->loadXML($xml, $options);
 
@@ -97,7 +84,6 @@ public static function fromString(
 
     /**
      * @param string $file
-     * @param string|null $schemaFile
      * @param non-negative-int $options
      *
      * @return \DOMDocument
@@ -117,9 +103,7 @@ public static function fromFile(
         }
 
         Assert::notWhitespaceOnly($xml, sprintf('File "%s" does not have content', $file), RuntimeException::class);
-        return (func_num_args() < 3)
-            ? static::fromString($xml, $schemaFile)
-            : static::fromString($xml, $schemaFile, $options);
+        return (func_num_args() < 2) ? static::fromString($xml) : static::fromString($xml, $options);
     }
 
 
@@ -132,39 +116,4 @@ public static function create(string $version = '1.0', string $encoding = 'UTF-8
     {
         return new DOMDocument($version, $encoding);
     }
-
-
-    /**
-     * Validate an XML-string against a given schema.
-     *
-     * @param string $xml
-     * @param string $schemaFile
-     * @param int $options
-     *
-     * @throws \SimpleSAML\XML\Exception\SchemaViolationException when validation fails.
-     */
-    public static function schemaValidation(
-        string $xml,
-        string $schemaFile,
-        int $options = self::DEFAULT_OPTIONS,
-    ): void {
-        if (!file_exists($schemaFile)) {
-            throw new IOException('File not found.');
-        }
-
-        $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;
-            }
-
-            throw new SchemaViolationException(sprintf(
-                "XML schema validation errors:\n - %s",
-                implode("\n - ", array_unique($msgs)),
-            ));
-        }
-    }
 }
diff --git a/src/SchemaValidatableElementInterface.php b/src/SchemaValidatableElementInterface.php
@@ -16,7 +16,7 @@ interface SchemaValidatableElementInterface extends ElementInterface
     /**
      * Validate the given DOMDocument against the schema set for this element
      *
-     * @return void
+     * @return \DOMDocument
      * @throws \SimpleSAML\XML\Exception\SchemaViolationException
      */
     public static function schemaValidate(DOMDocument $document): DOMDocument;

diff --git a/src/SchemaValidatableElementTrait.php b/src/SchemaValidatableElementTrait.php
@@ -6,16 +6,18 @@
 
 use DOMDocument;
 use SimpleSAML\Assert\Assert;
-use SimpleSAML\Exception\IOException;
-use SimpleSAML\Exception\SchemaViolationException;
+use SimpleSAML\XML\Exception\IOException;
+use SimpleSAML\XML\Exception\SchemaViolationException;
 
 use function array_unique;
 use function defined;
 use function file_exists;
 use function implode;
+use function libxml_get_errors;
+use function restore_error_handler;
+use function set_error_handler;
 use function sprintf;
 use function trim;
-use function libxml_get_errors;
 
 /**
  * trait class to be used by all the classes that implement the SchemaValidatableElementInterface
@@ -27,16 +29,28 @@ trait SchemaValidatableElementTrait
     /**
      * Validate the given DOMDocument against the schema set for this element
      *
-     * @return void
+     * @return \DOMDocument
      * @throws \SimpleSAML\XML\Exception\SchemaViolationException
      */
     public static function schemaValidate(DOMDocument $document): DOMDocument
     {
         $schemaFile = self::getSchemaFile();
-        $result = $document->schemaValidate($schemaFile);
 
+        // Dirty trick to catch the warnings emitted by XML-DOMs schemaValidate
+        // This will turn the warning into an exception
+        set_error_handler(static function (int $errno, string $errstr): never {
+            throw new SchemaViolationException($errstr, $errno);
+        }, E_WARNING);
+
+        try {
+            $result = $document->schemaValidate($schemaFile);
+        } finally {
+            // Restore the error handler, whether we throw an exception or not
+            restore_error_handler();
+        }
+
+        $msgs = [];
         if ($result === false) {
-            $msgs = [];
             foreach (libxml_get_errors() as $err) {
                 $msgs[] = trim($err->message) . ' on line ' . $err->line;
             }
@@ -53,6 +67,7 @@ public static function schemaValidate(DOMDocument $document): DOMDocument
 
     /**
      * Get the schema file that can validate this element.
+     * The path must be relative to the project's base directory.
      *
      * @return string
      */
@@ -62,7 +77,7 @@ public static function getSchemaFile(): string
             $schemaFile = static::SCHEMA;
         }
 
-        Assert::true(file_exists($schemaFile), IOException::class);
+        Assert::true(file_exists($schemaFile), sprintf("File not found: %s", $schemaFile), IOException::class);
         return $schemaFile;
     }
 }
diff --git a/src/TestUtils/SchemaValidationTestTrait.php b/src/TestUtils/SchemaValidationTestTrait.php
@@ -6,7 +6,6 @@
 
 use DOMDocument;
 use PHPUnit\Framework\Attributes\Depends;
-use SimpleSAML\XML\DOMDocumentFactory;
 
 use function class_exists;
 
@@ -20,9 +19,6 @@ trait SchemaValidationTestTrait
     /** @var class-string */
     protected static string $testedClass;
 
-    /** @var string */
-    protected static string $schemaFile;
-
     /** @var \DOMDocument */
     protected static DOMDocument $xmlRepresentation;
 
@@ -38,26 +34,21 @@ public function testSchemaValidation(): void
                 'Unable to run ' . self::class . '::testSchemaValidation(). Please set ' . self::class
                 . ':$testedClass to a class-string representing the XML-class being tested',
             );
-        } elseif (empty(self::$schemaFile)) {
-            $this->markTestSkipped(
-                'Unable to run ' . self::class . '::testSchemaValidation(). Please set ' . self::class
-                . ':$schema to point to a schema file',
-            );
         } elseif (empty(self::$xmlRepresentation)) {
             $this->markTestSkipped(
                 'Unable to run ' . self::class . '::testSchemaValidation(). Please set ' . self::class
                 . ':$xmlRepresentation to a DOMDocument representing the XML-class being tested',
             );
         } else {
             // Validate before serialization
-            DOMDocumentFactory::schemaValidation(self::$xmlRepresentation->saveXML(), self::$schemaFile);
+            self::$testedClass::schemaValidate(self::$xmlRepresentation);
 
             // Perform serialization
             $class = self::$testedClass::fromXML(self::$xmlRepresentation->documentElement);
             $serializedClass = $class->toXML();
 
             // Validate after serialization
-            DOMDocumentFactory::schemaValidation($serializedClass->ownerDocument->saveXML(), self::$schemaFile);
+            self::$testedClass::schemaValidate($serializedClass->ownerDocument);
 
             // If we got this far and no exceptions were thrown, consider this test passed!
             $this->addToAssertionCount(1);

diff --git a/tests/Utils/BooleanElement.php b/tests/Utils/BooleanElement.php
@@ -6,22 +6,29 @@
 
 use SimpleSAML\XML\AbstractElement;
 use SimpleSAML\XML\BooleanElementTrait;
+use SimpleSAML\XML\SchemaValidatableElementInterface;
+use SimpleSAML\XML\SchemaValidatableElementTrait;
 
 /**
  * Empty shell class for testing BooleanElement.
  *
  * @package simplesaml/xml-common
+ *
+ * Note: this class is not final for testing purposes.
  */
-final class BooleanElement extends AbstractElement
+class BooleanElement extends AbstractElement implements SchemaValidatableElementInterface
 {
     use BooleanElementTrait;
+    use SchemaValidatableElementTrait;
 
     /** @var string */
     public const NS = 'urn:x-simplesamlphp:namespace';
 
     /** @var string */
     public const NS_PREFIX = 'ssp';
 
+    public const SCHEMA = '/file/does/not/exist.xsd';
+
 
     /**
      * @param string $content

diff --git a/tests/Utils/ExtendableAttributesElement.php b/tests/Utils/ExtendableAttributesElement.php
@@ -9,16 +9,19 @@
 use SimpleSAML\XML\AbstractElement;
 use SimpleSAML\XML\Exception\InvalidDOMElementException;
 use SimpleSAML\XML\ExtendableAttributesTrait;
+use SimpleSAML\XML\SchemaValidatableElementInterface;
+use SimpleSAML\XML\SchemaValidatableElementTrait;
 use SimpleSAML\XML\XsNamespace as NS;
 
 /**
  * Empty shell class for testing ExtendableAttributesTrait.
  *
  * @package simplesaml/xml-security
  */
-class ExtendableAttributesElement extends AbstractElement
+class ExtendableAttributesElement extends AbstractElement implements SchemaValidatableElementInterface
 {
     use ExtendableAttributesTrait;
+    use SchemaValidatableElementTrait;
 
     /** @var string */
     public const NS = 'urn:x-simplesamlphp:namespace';
@@ -29,6 +32,9 @@ class ExtendableAttributesElement extends AbstractElement
     /** @var string */
     public const LOCALNAME = 'ExtendableAttributesElement';
 
+    /** @var string */
+    public const SCHEMA = 'tests/resources/schemas/simplesamlphp.xsd';
+
     /** @var string|\SimpleSAML\XML\XsNamespace */
     final public const XS_ANY_ATTR_NAMESPACE = NS::ANY;
 

diff --git a/tests/Utils/ExtendableElement.php b/tests/Utils/ExtendableElement.php
@@ -7,6 +7,8 @@
 use DOMElement;
 use SimpleSAML\XML\AbstractElement;
 use SimpleSAML\XML\ExtendableElementTrait;
+use SimpleSAML\XML\SchemaValidatableElementInterface;
+use SimpleSAML\XML\SchemaValidatableElementTrait;
 use SimpleSAML\XML\SerializableElementTrait;
 use SimpleSAML\XML\XsNamespace as NS;
 
@@ -15,9 +17,10 @@
  *
  * @package simplesaml/xml-security
  */
-class ExtendableElement extends AbstractElement
+class ExtendableElement extends AbstractElement implements SchemaValidatableElementInterface
 {
     use ExtendableElementTrait;
+    use SchemaValidatableElementTrait;
     use SerializableElementTrait;
 
     /** @var string */
@@ -29,6 +32,9 @@ class ExtendableElement extends AbstractElement
     /** @var string */
     public const LOCALNAME = 'ExtendableElement';
 
+    /** @var string */
+    public const SCHEMA = 'tests/resources/schemas/simplesamlphp.xsd';
+
     /** @var \SimpleSAML\XML\XsNamespace|array<int, \SimpleSAML\XML\XsNamespace> */
     final public const XS_ANY_ELT_NAMESPACE = NS::ANY;
 

diff --git a/tests/Utils/StringElement.php b/tests/Utils/StringElement.php
@@ -5,15 +5,18 @@
 namespace SimpleSAML\Test\XML;
 
 use SimpleSAML\XML\AbstractElement;
+use SimpleSAML\XML\SchemaValidatableElementInterface;
+use SimpleSAML\XML\SchemaValidatableElementTrait;
 use SimpleSAML\XML\StringElementTrait;
 
 /**
  * Empty shell class for testing String elements.
  *
  * @package simplesaml/xml-common
  */
-final class StringElement extends AbstractElement
+final class StringElement extends AbstractElement implements SchemaValidatableElementInterface
 {
+    use SchemaValidatableElementTrait;
     use StringElementTrait;
 
     /** @var string */
@@ -22,6 +25,9 @@ final class StringElement extends AbstractElement
     /** @var string */
     public const NS_PREFIX = 'ssp';
 
+    /** @var string */
+    public const SCHEMA = 'tests/resources/schemas/simplesamlphp.xsd';
+
 
     /**
      * @param string $content