From b21a8cfc3a8b0e497d87ed21e4c1d656d997e9fb Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 2 Jun 2026 16:45:42 +0000 Subject: [PATCH 1/4] fix SchemaValidatableElementTrait::schemaValidate inconsistent behavior across modules. Fix bugs. --- src/XML/DOMDocumentFactory.php | 4 +- src/XML/SchemaValidatableElementTrait.php | 61 ++++++++++++++++------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/src/XML/DOMDocumentFactory.php b/src/XML/DOMDocumentFactory.php index f767666d..00089f12 100644 --- a/src/XML/DOMDocumentFactory.php +++ b/src/XML/DOMDocumentFactory.php @@ -97,7 +97,7 @@ static function (int $severity, string $message): never { restore_error_handler(); } - foreach ($domDocument->childNodes as $child) { + foreach ($loaded->childNodes as $child) { Assert::false( $child->nodeType === \XML_DOCUMENT_TYPE_NODE, 'Dangerous XML detected, DOCTYPE nodes are not allowed in the XML body', @@ -245,7 +245,7 @@ public static function lookupNamespaceURI(Dom\Element $elt, ?string $prefix): ?s } /** @var \Dom\NamespaceInfo[] $namespaces */ - $namespaces = $elt->ownerDocument->documentElement->getInScopeNamespaces(); + $namespaces = $elt->getInScopeNamespaces(); foreach ($namespaces as $ns) { if ($ns->prefix === $prefix) { diff --git a/src/XML/SchemaValidatableElementTrait.php b/src/XML/SchemaValidatableElementTrait.php index 6a76daf1..dca0a167 100644 --- a/src/XML/SchemaValidatableElementTrait.php +++ b/src/XML/SchemaValidatableElementTrait.php @@ -5,6 +5,7 @@ namespace SimpleSAML\XML; use Dom; +use Dom\XMLDocument; use SimpleSAML\XML\Assert\Assert; use SimpleSAML\XML\Exception\IOException; use SimpleSAML\XML\Exception\RuntimeException; @@ -41,29 +42,55 @@ public static function schemaValidate(Dom\Document $document, ?string $schemaFil $internalErrors = libxml_use_internal_errors(true); libxml_clear_errors(); - if ($schemaFile === null) { - $schemaFile = self::getSchemaFile(); - } + try { + if ($schemaFile === null) { + $schemaFile = self::getSchemaFile(); + } - // Must suppress the warnings here in order to throw them as an error below. - $result = @$document->schemaValidate($schemaFile); + if (!$document instanceof XMLDocument) { + throw new \LogicException('Schema validation requires an instance of Dom\\XMLDocument.'); + } - if ($result === false) { - $msgs = []; - foreach (libxml_get_errors() as $err) { - $msgs[] = trim($err->message) . ' on line ' . $err->line; + /** + * Validates using a legacy \DOMDocument round-trip (serialize + re-parse) before running schema validation. + * This avoids false negatives seen with Dom\Document::schemaValidate(), especially around xs:QName + * prefix scope. Validation is performed against the exact serialized XML that would be + * exchanged externally. + */ + $root = $document->documentElement; + if ($root === null) { + throw new SchemaViolationException('The document has no document element.'); } - throw new SchemaViolationException(sprintf( - "XML schema validation errors:\n - %s", - implode("\n - ", array_unique($msgs)), - )); - } + $xml = $document->saveXml($root); + if ($xml === false || trim($xml) === '') { + throw new SchemaViolationException('Could not serialize XML for schema validation.'); + } - libxml_use_internal_errors($internalErrors); - libxml_clear_errors(); + $legacy = new \DOMDocument('1.0', 'UTF-8'); + $legacy->preserveWhiteSpace = true; + $legacy->formatOutput = false; + $legacy->loadXML($xml); - return $document; + $result = $legacy->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)), + )); + } + + return $document; + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($internalErrors); + } } From 675c3392df37527f02041d8acba1b5a2a85bc539 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 2 Jun 2026 17:00:28 +0000 Subject: [PATCH 2/4] improve DOMDocument phpdoc usage reasoning --- src/XML/SchemaValidatableElementTrait.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/XML/SchemaValidatableElementTrait.php b/src/XML/SchemaValidatableElementTrait.php index dca0a167..3da49ed1 100644 --- a/src/XML/SchemaValidatableElementTrait.php +++ b/src/XML/SchemaValidatableElementTrait.php @@ -51,12 +51,6 @@ public static function schemaValidate(Dom\Document $document, ?string $schemaFil throw new \LogicException('Schema validation requires an instance of Dom\\XMLDocument.'); } - /** - * Validates using a legacy \DOMDocument round-trip (serialize + re-parse) before running schema validation. - * This avoids false negatives seen with Dom\Document::schemaValidate(), especially around xs:QName - * prefix scope. Validation is performed against the exact serialized XML that would be - * exchanged externally. - */ $root = $document->documentElement; if ($root === null) { throw new SchemaViolationException('The document has no document element.'); @@ -67,6 +61,14 @@ public static function schemaValidate(Dom\Document $document, ?string $schemaFil throw new SchemaViolationException('Could not serialize XML for schema validation.'); } + /** + * Validates using a legacy \DOMDocument round-trip (serialize + re-parse) before running schema validation. + * This avoids false negatives seen with Dom\Document::schemaValidate(), especially around xs:QName + * prefix scope. Validation is performed against the exact serialized XML that would be + * exchanged externally. + * TODO: Revisit when Dom\Document::schemaValidate() reliably handles xs:QName prefix scope + * (avoid legacy \DOMDocument round-trip). + */ $legacy = new \DOMDocument('1.0', 'UTF-8'); $legacy->preserveWhiteSpace = true; $legacy->formatOutput = false; From 3b127f810628a9231d6c7c7ea05052095f519bc2 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 2 Jun 2026 17:04:16 +0000 Subject: [PATCH 3/4] fix review suggestions/comments --- src/XML/SchemaValidatableElementTrait.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/XML/SchemaValidatableElementTrait.php b/src/XML/SchemaValidatableElementTrait.php index 3da49ed1..ea2e625a 100644 --- a/src/XML/SchemaValidatableElementTrait.php +++ b/src/XML/SchemaValidatableElementTrait.php @@ -5,7 +5,7 @@ namespace SimpleSAML\XML; use Dom; -use Dom\XMLDocument; +use LogicException; use SimpleSAML\XML\Assert\Assert; use SimpleSAML\XML\Exception\IOException; use SimpleSAML\XML\Exception\RuntimeException; @@ -47,8 +47,8 @@ public static function schemaValidate(Dom\Document $document, ?string $schemaFil $schemaFile = self::getSchemaFile(); } - if (!$document instanceof XMLDocument) { - throw new \LogicException('Schema validation requires an instance of Dom\\XMLDocument.'); + if (!$document instanceof Dom\XMLDocument) { + throw new LogicException('Schema validation requires an instance of Dom\\XMLDocument.'); } $root = $document->documentElement; From e07679f2beb6a39d37aeddc30bdf74e21adf8076 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Wed, 3 Jun 2026 17:48:50 +0000 Subject: [PATCH 4/4] revert SchemaValidatableElementTrait change --- src/XML/SchemaValidatableElementTrait.php | 63 ++++++----------------- 1 file changed, 17 insertions(+), 46 deletions(-) diff --git a/src/XML/SchemaValidatableElementTrait.php b/src/XML/SchemaValidatableElementTrait.php index ea2e625a..6a76daf1 100644 --- a/src/XML/SchemaValidatableElementTrait.php +++ b/src/XML/SchemaValidatableElementTrait.php @@ -5,7 +5,6 @@ namespace SimpleSAML\XML; use Dom; -use LogicException; use SimpleSAML\XML\Assert\Assert; use SimpleSAML\XML\Exception\IOException; use SimpleSAML\XML\Exception\RuntimeException; @@ -42,57 +41,29 @@ public static function schemaValidate(Dom\Document $document, ?string $schemaFil $internalErrors = libxml_use_internal_errors(true); libxml_clear_errors(); - try { - if ($schemaFile === null) { - $schemaFile = self::getSchemaFile(); - } - - if (!$document instanceof Dom\XMLDocument) { - throw new LogicException('Schema validation requires an instance of Dom\\XMLDocument.'); - } + if ($schemaFile === null) { + $schemaFile = self::getSchemaFile(); + } - $root = $document->documentElement; - if ($root === null) { - throw new SchemaViolationException('The document has no document element.'); - } + // Must suppress the warnings here in order to throw them as an error below. + $result = @$document->schemaValidate($schemaFile); - $xml = $document->saveXml($root); - if ($xml === false || trim($xml) === '') { - throw new SchemaViolationException('Could not serialize XML for schema validation.'); + if ($result === false) { + $msgs = []; + foreach (libxml_get_errors() as $err) { + $msgs[] = trim($err->message) . ' on line ' . $err->line; } - /** - * Validates using a legacy \DOMDocument round-trip (serialize + re-parse) before running schema validation. - * This avoids false negatives seen with Dom\Document::schemaValidate(), especially around xs:QName - * prefix scope. Validation is performed against the exact serialized XML that would be - * exchanged externally. - * TODO: Revisit when Dom\Document::schemaValidate() reliably handles xs:QName prefix scope - * (avoid legacy \DOMDocument round-trip). - */ - $legacy = new \DOMDocument('1.0', 'UTF-8'); - $legacy->preserveWhiteSpace = true; - $legacy->formatOutput = false; - $legacy->loadXML($xml); - - $result = $legacy->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)), + )); + } - throw new SchemaViolationException(sprintf( - "XML schema validation errors:\n - %s", - implode("\n - ", array_unique($msgs)), - )); - } + libxml_use_internal_errors($internalErrors); + libxml_clear_errors(); - return $document; - } finally { - libxml_clear_errors(); - libxml_use_internal_errors($internalErrors); - } + return $document; }