From a71e04bd9984cf86ca413ef5d4ab9b491550f10e Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Thu, 13 Nov 2025 10:36:33 +0200 Subject: [PATCH 1/7] xpath ancestor registration --- composer.json | 22 +++- src/XPath/XPath.php | 65 ++++++++++++ tests/XML/ExtendableAttributesTraitTest.php | 1 - tests/XML/ExtendableElementTraitTest.php | 2 - tests/XPath/XPathTest.php | 108 ++++++++++++++++++++ 5 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 tests/XPath/XPathTest.php diff --git a/composer.json b/composer.json index f0b370cc..553a177a 100644 --- a/composer.json +++ b/composer.json @@ -42,8 +42,7 @@ "ext-pcre": "*", "ext-spl": "*", - "simplesamlphp/assert": "~1.9.0", - "simplesamlphp/composer-xmlprovider-installer": "~1.1.0" + "simplesamlphp/assert": "~1.9.0" }, "require-dev": { "simplesamlphp/simplesamlphp-test-framework": "~1.10.1" @@ -56,8 +55,23 @@ "allow-plugins": { "composer/package-versions-deprecated": true, "dealerdirect/phpcodesniffer-composer-installer": true, - "phpstan/extension-installer": true, - "simplesamlphp/composer-xmlprovider-installer": true + "phpstan/extension-installer": true } + }, + "scripts": { + "pre-commit": [ + "vendor/bin/phpcs -p", + "vendor/bin/composer-require-checker check --config-file=tools/composer-require-checker.json composer.json", + "vendor/bin/phpstan analyze -c phpstan.neon", + "vendor/bin/phpstan analyze -c phpstan-dev.neon", + "vendor/bin/composer-unused", + "vendor/bin/phpunit --no-coverage --testdox" + ], + "tests": [ + "vendor/bin/phpunit --no-coverage" + ], + "propose-fix": [ + "vendor/bin/phpcs --report=diff" + ] } } diff --git a/src/XPath/XPath.php b/src/XPath/XPath.php index 0a400c63..7680721d 100644 --- a/src/XPath/XPath.php +++ b/src/XPath/XPath.php @@ -5,6 +5,7 @@ namespace SimpleSAML\XPath; use DOMDocument; +use DOMElement; use DOMNode; use DOMXPath; use SimpleSAML\XML\Assert\Assert; @@ -21,6 +22,12 @@ class XPath /** * Get an instance of DOMXPath associated with a DOMNode * + * - Reuses a cached DOMXPath per document. + * - Registers core XML-related namespaces: 'xml' and 'xs'. + * - Enriches the XPath with all prefixed xmlns declarations found on the + * current node and its ancestors (up to the document element), so + * custom prefixes declared anywhere up the tree can be used in queries. + * * @param \DOMNode $node The associated node * @return \DOMXPath */ @@ -42,10 +49,68 @@ public static function getXPath(DOMNode $node): DOMXPath $xpCache->registerNamespace('xml', C_XML::NS_XML); $xpCache->registerNamespace('xs', C_XS::NS_XS); + // Enrich with ancestor-declared prefixes for this document context. + self::registerAncestorNamespaces($xpCache, $node); + return $xpCache; } + /** + * Walk from the given node up to the document element, registering all prefixed xmlns declarations. + * + * Safety: + * - Only attributes in the XMLNS namespace (http://www.w3.org/2000/xmlns/). + * - Skip default xmlns (localName === 'xmlns') because XPath requires prefixes. + * - Skip empty URIs. + * - Do not override core 'xml' and 'xs' prefixes (already bound). + * - Nearest binding wins during this pass (prefixes are added once). + * + * @param \DOMXPath $xp + * @param \DOMNode $node + */ + private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): void + { + $xmlnsNs = 'http://www.w3.org/2000/xmlns/'; + + // Avoid re-binding while walking upwards. + $registered = [ + 'xml' => true, + 'xs' => true, + ]; + + // Start from the nearest element (or documentElement if a DOMDocument is passed). + $current = $node instanceof DOMDocument + ? $node->documentElement + : ($node instanceof DOMElement ? $node : $node->parentNode); + + while ($current instanceof DOMElement) { + if ($current->hasAttributes()) { + foreach ($current->attributes as $attr) { + if ($attr->namespaceURI !== $xmlnsNs) { + continue; + } + $prefix = $attr->localName; // e.g., 'slate' for xmlns:slate, 'xmlns' for default + $uri = (string) $attr->nodeValue; + + if ( + $prefix === null || $prefix === '' || + $prefix === 'xmlns' || $uri === '' || + isset($registered[$prefix]) + ) { + continue; + } + + $xp->registerNamespace($prefix, $uri); + $registered[$prefix] = true; + } + } + + $current = $current->parentNode instanceof DOMElement ? $current->parentNode : null; + } + } + + /** * Do an XPath query on an XML node. * diff --git a/tests/XML/ExtendableAttributesTraitTest.php b/tests/XML/ExtendableAttributesTraitTest.php index 36fe803c..77b65936 100644 --- a/tests/XML/ExtendableAttributesTraitTest.php +++ b/tests/XML/ExtendableAttributesTraitTest.php @@ -88,7 +88,6 @@ public function getAttributeNamespace(): array|string public function testEmptyNamespaceArrayThrowsAnException(): void { $this->expectException(AssertionFailedException::class); - // @phpstan-ignore expr.resultUnused new class ([]) extends ExtendableAttributesElement { /** * @return array|string diff --git a/tests/XML/ExtendableElementTraitTest.php b/tests/XML/ExtendableElementTraitTest.php index b7255c28..5eff7adb 100644 --- a/tests/XML/ExtendableElementTraitTest.php +++ b/tests/XML/ExtendableElementTraitTest.php @@ -85,7 +85,6 @@ public static function setUpBeforeClass(): void public function testIllegalNamespaceComboThrowsAnException(): void { $this->expectException(AssertionFailedException::class); - // @phpstan-ignore expr.resultUnused new class ([]) extends ExtendableElement { /** * @return array|string @@ -104,7 +103,6 @@ public function getElementNamespace(): array|string public function testEmptyNamespaceArrayThrowsAnException(): void { $this->expectException(AssertionFailedException::class); - // @phpstan-ignore expr.resultUnused new class ([]) extends ExtendableElement { /** * @return array|string diff --git a/tests/XPath/XPathTest.php b/tests/XPath/XPathTest.php new file mode 100644 index 00000000..e8152aec --- /dev/null +++ b/tests/XPath/XPathTest.php @@ -0,0 +1,108 @@ +loadXML(<<<'XML' + + + value + +XML); + + // Doc B is different + $docB = new \DOMDocument(); + $docB->loadXML(<<<'XML' + + +XML); + + $xpA1 = XPath::getXPath($docA); + $xpA2 = XPath::getXPath($docA); + $xpB = XPath::getXPath($docB); + + // Cached instance reused per same document + $this->assertSame($xpA1, $xpA2); + // Different document => different DOMXPath instance + $this->assertNotSame($xpA1, $xpB); + + // 'xml' prefix registered: query should be valid and return xml:space attribute + $rootA = $docA->documentElement; + $this->assertInstanceOf(\DOMElement::class, $rootA); + $attrs = XPath::xpQuery($rootA, '@xml:space', $xpA1); + $this->assertCount(1, $attrs); + $this->assertSame('preserve', $attrs[0]->nodeValue); + } + + + public function testAncestorNamespaceRegistrationAllowsCustomPrefixes(): void + { + // Custom namespace declared on the root; query from a descendant node + $xml = <<<'XML' + + + + + ok + + + +XML; + $doc = new \DOMDocument(); + $doc->loadXML($xml); + + // Use a deep context node to ensure ancestor-walk picks up xmlns:foo from root + $context = $doc->getElementsByTagName('b')->item(0); + $this->assertInstanceOf(\DOMElement::class, $context); + + $xp = XPath::getXPath($context); + + $nodes = XPath::xpQuery($context, 'foo:item', $xp); + $this->assertCount(1, $nodes); + $this->assertSame('ok', $nodes[0]->textContent); + } + + + public function testXpQueryThrowsOnMalformedExpression(): void + { + $doc = new \DOMDocument(); + $doc->loadXML(''); + $xp = XPath::getXPath($doc); + + // If xpQuery throws a specific exception, put that class here instead of \Throwable. + $this->expectException(\Throwable::class); + // Keep message assertion resilient to libxml version differences. + $this->expectExceptionMessageMatches('/(XPath|expression).*invalid|malformed|error/i'); + + // Malformed XPath: missing closing bracket + $root = $doc->documentElement; + $this->assertInstanceOf(\DOMElement::class, $root); + + // Avoid emitting a PHP warning; let xpQuery surface it as an exception. + \libxml_use_internal_errors(true); + try { + XPath::xpQuery($root, '//*[', $xp); + } finally { + $errors = \libxml_get_errors(); + self::assertCount(1, $errors); + self::assertEquals("Invalid expression\n", $errors[0]->message); + \libxml_clear_errors(); + \libxml_use_internal_errors(false); + } + } +} From e6b1f2c879953032ec6893002e3e1b33295456d7 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Thu, 13 Nov 2025 10:44:04 +0200 Subject: [PATCH 2/7] Move xmlns to constants class --- src/XML/Constants.php | 5 +++++ src/XPath/XPath.php | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/XML/Constants.php b/src/XML/Constants.php index 96b2f4a0..6903702c 100644 --- a/src/XML/Constants.php +++ b/src/XML/Constants.php @@ -16,6 +16,11 @@ class Constants */ public const NS_XML = 'http://www.w3.org/XML/1998/namespace'; + /** + * The namespace for XMLNS declarations. + */ + public const NS_XMLNS = 'http://www.w3.org/2000/xmlns/'; + /** * The maximum amount of child nodes this library is willing to handle. * By specification, this limit is 150K, but that opens up for denial of service. diff --git a/src/XPath/XPath.php b/src/XPath/XPath.php index 7680721d..a946e2f1 100644 --- a/src/XPath/XPath.php +++ b/src/XPath/XPath.php @@ -71,8 +71,6 @@ public static function getXPath(DOMNode $node): DOMXPath */ private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): void { - $xmlnsNs = 'http://www.w3.org/2000/xmlns/'; - // Avoid re-binding while walking upwards. $registered = [ 'xml' => true, @@ -87,7 +85,7 @@ private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): while ($current instanceof DOMElement) { if ($current->hasAttributes()) { foreach ($current->attributes as $attr) { - if ($attr->namespaceURI !== $xmlnsNs) { + if ($attr->namespaceURI !== C_XML::NS_XMLNS) { continue; } $prefix = $attr->localName; // e.g., 'slate' for xmlns:slate, 'xmlns' for default From c5367f03131c0b8f111a7461bfca006fcb3fc462 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Thu, 13 Nov 2025 10:53:51 +0200 Subject: [PATCH 3/7] Improve XPathTest.php imports/use statements --- tests/XPath/XPathTest.php | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tests/XPath/XPathTest.php b/tests/XPath/XPathTest.php index e8152aec..f4420190 100644 --- a/tests/XPath/XPathTest.php +++ b/tests/XPath/XPathTest.php @@ -4,9 +4,15 @@ namespace SimpleSAML\Test\XPath; +use DOMDocument; +use DOMElement; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use SimpleSAML\XPath\XPath; +use Throwable; + +use function libxml_clear_errors; +use function libxml_use_internal_errors; /** * Tests for the SimpleSAML\XPath\XPath helper. @@ -17,7 +23,7 @@ final class XPathTest extends TestCase public function testGetXPathCachesPerDocumentAndRegistersCoreNamespaces(): void { // Doc A with an xml:space attribute to validate 'xml' prefix usage works - $docA = new \DOMDocument(); + $docA = new DOMDocument(); $docA->loadXML(<<<'XML' @@ -26,7 +32,7 @@ public function testGetXPathCachesPerDocumentAndRegistersCoreNamespaces(): void XML); // Doc B is different - $docB = new \DOMDocument(); + $docB = new DOMDocument(); $docB->loadXML(<<<'XML' @@ -43,7 +49,7 @@ public function testGetXPathCachesPerDocumentAndRegistersCoreNamespaces(): void // 'xml' prefix registered: query should be valid and return xml:space attribute $rootA = $docA->documentElement; - $this->assertInstanceOf(\DOMElement::class, $rootA); + $this->assertInstanceOf(DOMElement::class, $rootA); $attrs = XPath::xpQuery($rootA, '@xml:space', $xpA1); $this->assertCount(1, $attrs); $this->assertSame('preserve', $attrs[0]->nodeValue); @@ -63,7 +69,7 @@ public function testAncestorNamespaceRegistrationAllowsCustomPrefixes(): void XML; - $doc = new \DOMDocument(); + $doc = new DOMDocument(); $doc->loadXML($xml); // Use a deep context node to ensure ancestor-walk picks up xmlns:foo from root @@ -80,29 +86,29 @@ public function testAncestorNamespaceRegistrationAllowsCustomPrefixes(): void public function testXpQueryThrowsOnMalformedExpression(): void { - $doc = new \DOMDocument(); + $doc = new DOMDocument(); $doc->loadXML(''); $xp = XPath::getXPath($doc); - // If xpQuery throws a specific exception, put that class here instead of \Throwable. - $this->expectException(\Throwable::class); + // If xpQuery throws a specific exception, put that class here instead of Throwable. + $this->expectException(Throwable::class); // Keep message assertion resilient to libxml version differences. $this->expectExceptionMessageMatches('/(XPath|expression).*invalid|malformed|error/i'); // Malformed XPath: missing closing bracket $root = $doc->documentElement; - $this->assertInstanceOf(\DOMElement::class, $root); + $this->assertInstanceOf(DOMElement::class, $root); // Avoid emitting a PHP warning; let xpQuery surface it as an exception. - \libxml_use_internal_errors(true); + libxml_use_internal_errors(true); try { XPath::xpQuery($root, '//*[', $xp); } finally { - $errors = \libxml_get_errors(); + $errors = libxml_get_errors(); self::assertCount(1, $errors); self::assertEquals("Invalid expression\n", $errors[0]->message); - \libxml_clear_errors(); - \libxml_use_internal_errors(false); + libxml_clear_errors(); + libxml_use_internal_errors(false); } } } From fd976e5758e4d0929d9aa708dcdd902d75aa2ab1 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Thu, 13 Nov 2025 11:13:14 +0200 Subject: [PATCH 4/7] restore composer xmlprovider installer --- composer.json | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 553a177a..6e4ef522 100644 --- a/composer.json +++ b/composer.json @@ -45,7 +45,8 @@ "simplesamlphp/assert": "~1.9.0" }, "require-dev": { - "simplesamlphp/simplesamlphp-test-framework": "~1.10.1" + "simplesamlphp/simplesamlphp-test-framework": "~1.10.1", + "simplesamlphp/composer-xmlprovider-installer": "~1.1.0" }, "support": { "issues": "https://github.com/simplesamlphp/xml-common/issues", @@ -55,7 +56,8 @@ "allow-plugins": { "composer/package-versions-deprecated": true, "dealerdirect/phpcodesniffer-composer-installer": true, - "phpstan/extension-installer": true + "phpstan/extension-installer": true, + "simplesamlphp/composer-xmlprovider-installer": true } }, "scripts": { @@ -64,7 +66,7 @@ "vendor/bin/composer-require-checker check --config-file=tools/composer-require-checker.json composer.json", "vendor/bin/phpstan analyze -c phpstan.neon", "vendor/bin/phpstan analyze -c phpstan-dev.neon", - "vendor/bin/composer-unused", + "vendor/bin/composer-unused --excludePackage=simplesamlphp/composer-xmlprovider-installer", "vendor/bin/phpunit --no-coverage --testdox" ], "tests": [ From 337d83b5903eff6e55a22db60d3292e564c93cd0 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Thu, 13 Nov 2025 11:18:40 +0200 Subject: [PATCH 5/7] fix dependency path --- composer.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 6e4ef522..2edf5a7c 100644 --- a/composer.json +++ b/composer.json @@ -42,11 +42,11 @@ "ext-pcre": "*", "ext-spl": "*", - "simplesamlphp/assert": "~1.9.0" + "simplesamlphp/assert": "~1.9.0", + "simplesamlphp/composer-xmlprovider-installer": "~1.1.0" }, "require-dev": { - "simplesamlphp/simplesamlphp-test-framework": "~1.10.1", - "simplesamlphp/composer-xmlprovider-installer": "~1.1.0" + "simplesamlphp/simplesamlphp-test-framework": "~1.10.1" }, "support": { "issues": "https://github.com/simplesamlphp/xml-common/issues", From 8f9ac48733f1b12645c142d7fdae0bfb9268c3cc Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 14 Nov 2025 10:49:33 +0200 Subject: [PATCH 6/7] Improve testing.Add descendants node access. --- src/XPath/XPath.php | 107 +++++++++- tests/XPath/XPathTest.php | 221 +++++++++++++++++++++ tests/resources/xml/success_response_a.xml | 21 ++ tests/resources/xml/success_response_b.xml | 20 ++ 4 files changed, 359 insertions(+), 10 deletions(-) create mode 100644 tests/resources/xml/success_response_a.xml create mode 100644 tests/resources/xml/success_response_b.xml diff --git a/src/XPath/XPath.php b/src/XPath/XPath.php index a946e2f1..c4d47955 100644 --- a/src/XPath/XPath.php +++ b/src/XPath/XPath.php @@ -8,6 +8,7 @@ use DOMElement; use DOMNode; use DOMXPath; +use RuntimeException; use SimpleSAML\XML\Assert\Assert; use SimpleSAML\XML\Constants as C_XML; use SimpleSAML\XMLSchema\Constants as C_XS; @@ -50,7 +51,10 @@ public static function getXPath(DOMNode $node): DOMXPath $xpCache->registerNamespace('xs', C_XS::NS_XS); // Enrich with ancestor-declared prefixes for this document context. - self::registerAncestorNamespaces($xpCache, $node); + $prefixToUri = self::registerAncestorNamespaces($xpCache, $node); + + // Single, bounded subtree scan to pick up descendant-only declarations. + self::registerSubtreePrefixes($xpCache, $node, $prefixToUri); return $xpCache; } @@ -68,13 +72,14 @@ public static function getXPath(DOMNode $node): DOMXPath * * @param \DOMXPath $xp * @param \DOMNode $node + * @return array Map of prefix => namespace URI that are bound after this pass */ - private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): void + private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): array { - // Avoid re-binding while walking upwards. - $registered = [ - 'xml' => true, - 'xs' => true, + // Track prefix => uri to feed into subtree scan. Seed with core bindings. + $prefixToUri = [ + 'xml' => C_XML::NS_XML, + 'xs' => C_XS::NS_XS, ]; // Start from the nearest element (or documentElement if a DOMDocument is passed). @@ -82,29 +87,111 @@ private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): ? $node->documentElement : ($node instanceof DOMElement ? $node : $node->parentNode); + $steps = 0; + while ($current instanceof DOMElement) { + if (++$steps > C_XML::UNBOUNDED_LIMIT) { + throw new RuntimeException(__METHOD__ . ': exceeded ancestor traversal limit'); + } + if ($current->hasAttributes()) { foreach ($current->attributes as $attr) { if ($attr->namespaceURI !== C_XML::NS_XMLNS) { continue; } - $prefix = $attr->localName; // e.g., 'slate' for xmlns:slate, 'xmlns' for default + $prefix = $attr->localName; $uri = (string) $attr->nodeValue; if ( $prefix === null || $prefix === '' || $prefix === 'xmlns' || $uri === '' || - isset($registered[$prefix]) + isset($prefixToUri[$prefix]) ) { continue; } $xp->registerNamespace($prefix, $uri); - $registered[$prefix] = true; + $prefixToUri[$prefix] = $uri; + } + } + + $current = $current->parentNode; + } + + return $prefixToUri; + } + + + /** + * Single-pass subtree scan from the context element to bind prefixes used only on descendants. + * - Never rebind an already-registered prefix (collision-safe). + * - Skips 'xmlns' and empty URIs. + * - Bounded by UNBOUNDED_LIMIT. + * + * @param \DOMXPath $xp + * @param \DOMNode $node + * @param array $prefixToUri + */ + private static function registerSubtreePrefixes(DOMXPath $xp, DOMNode $node, array $prefixToUri): void + { + $root = $node instanceof DOMDocument + ? $node->documentElement + : ($node instanceof DOMElement ? $node : $node->parentNode); + + if (!$root instanceof DOMElement) { + return; + } + + $visited = 0; + + /** @var array<\DOMElement> $queue */ + $queue = [$root]; + + while ($queue) { + /** @var \DOMElement $el */ + $el = array_shift($queue); + + if (++$visited > C_XML::UNBOUNDED_LIMIT) { + throw new \RuntimeException(__METHOD__ . ': exceeded subtree traversal limit'); + } + + // Element prefix + if ($el->prefix && !isset($prefixToUri[$el->prefix])) { + $uri = $el->namespaceURI; + if (is_string($uri) && $uri !== '') { + $xp->registerNamespace($el->prefix, $uri); + $prefixToUri[$el->prefix] = $uri; } } - $current = $current->parentNode instanceof DOMElement ? $current->parentNode : null; + // Attribute prefixes (excluding xmlns) + if ($el->hasAttributes()) { + foreach ($el->attributes as $attr) { + if ( + $attr->prefix && + $attr->prefix !== 'xmlns' && + !isset($prefixToUri[$attr->prefix]) + ) { + $uri = $attr->namespaceURI; + if (is_string($uri) && $uri !== '') { + $xp->registerNamespace($attr->prefix, $uri); + $prefixToUri[$attr->prefix] = $uri; + } + } else { + // Optional: collision detection (same prefix, different URI) + // if ($prefixToUri[$pfx] !== $attr->namespaceURI) { + // // Default: skip rebind; could log a debug message here. + // } + } + } + } + + // Enqueue children (only DOMElement to keep types precise) + foreach ($el->childNodes as $child) { + if ($child instanceof DOMElement) { + $queue[] = $child; + } + } } } diff --git a/tests/XPath/XPathTest.php b/tests/XPath/XPathTest.php index f4420190..11f0aa1c 100644 --- a/tests/XPath/XPathTest.php +++ b/tests/XPath/XPathTest.php @@ -6,8 +6,11 @@ use DOMDocument; use DOMElement; +use DOMText; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use SimpleSAML\XML\DOMDocumentFactory; use SimpleSAML\XPath\XPath; use Throwable; @@ -111,4 +114,222 @@ public function testXpQueryThrowsOnMalformedExpression(): void libxml_use_internal_errors(false); } } + + + public function testXmlnsDetectionRegistersPrefixedNamespace(): void + { + // This XML ensures we hit the detection branch and pass the second guard: + // - xmlns:foo="urn:two" should be detected and registered + $xml = <<<'XML' + + + + ok + + +XML; + + $doc = new DOMDocument(); + $doc->loadXML($xml); + + $context = $doc->getElementsByTagName('ctx')->item(0); + $this->assertInstanceOf(DOMElement::class, $context); + + $xp = XPath::getXPath($context); + + // Passing the guards should register 'foo' so this resolves + $nodes = XPath::xpQuery($context, 'foo:item', $xp); + $this->assertCount(1, $nodes); + $this->assertSame('ok', $nodes[0]->textContent); + } + + + public function testNormalizationFromTextNode(): void + { + $xml = <<<'XML' + + + ok + +XML; + $doc = new DOMDocument(); + $doc->loadXML($xml); + + $item = $doc->getElementsByTagNameNS('https://example.org/foo', 'item')->item(0); + $this->assertInstanceOf(DOMElement::class, $item); + + $text = $item->firstChild; // DOMText node inside + $this->assertInstanceOf(DOMText::class, $text); + + // getXPath should handle a non-element node, normalize to the nearest element ancestor, + // and register the 'foo' namespace so a prefixed query works. + $xp = XPath::getXPath($text); + + $nodes = XPath::xpQuery($text, 'ancestor::foo:item', $xp); + $this->assertCount(1, $nodes); + $this->assertSame('ok', $nodes[0]->textContent); + } + + + public function testNormalizationFromAttributeNode(): void + { + $xml = <<<'XML' + + + + +XML; + $doc = new DOMDocument(); + $doc->loadXML($xml); + + $elt = $doc->getElementsByTagNameNS('urn:bar', 'elt')->item(0); + $this->assertInstanceOf(\DOMElement::class, $elt); + + $attr = $elt->getAttributeNodeNS('urn:bar', 'attr'); + $this->assertInstanceOf(\DOMAttr::class, $attr); + + // getXPath should normalize from DOMAttr to the element and ensure 'bar' is registered. + $xp = XPath::getXPath($attr); + + $nodes = XPath::xpQuery($attr, 'ancestor::bar:elt', $xp); + $this->assertCount(1, $nodes); + + // Ensure we have an element before calling element-only methods. + $this->assertInstanceOf(\DOMElement::class, $nodes[0]); + /** @var \DOMElement $el */ + $el = $nodes[0]; + $this->assertSame('v', $el->getAttributeNS('urn:bar', 'attr')); + } + + + public function testSkipsDefaultNamespaceDeclarationDoesNotCreateUsableXmlnsPrefix(): void + { + // Default namespace present; no prefixed declaration. + // The guard should skip registering 'xmlns' as a usable prefix. + $xml = <<<'XML' + + + t + +XML; + $doc = new DOMDocument(); + $doc->loadXML($xml); + + $context = $doc->documentElement?->getElementsByTagName('b')->item(0); + $this->assertInstanceOf(\DOMElement::class, $context); + + $xp = XPath::getXPath($context); + + // Using 'xmlns' as a prefix should fail because the code skips binding it. + libxml_use_internal_errors(true); + try { + $this->expectException(\Throwable::class); + // The XPath helper wraps libxml errors into a generic message: + $this->expectExceptionMessage('Malformed XPath query or invalid contextNode provided.'); + XPath::xpQuery($context, 'xmlns:b', $xp); + } finally { + $errors = libxml_get_errors(); + $this->assertEquals("Undefined namespace prefix\n", $errors[0]->message); + libxml_clear_errors(); + libxml_use_internal_errors(false); + } + } + + + public function testSkipsEmptyUriNamespaceDeclaration(): void + { + // An empty-URI namespace declaration must be ignored by the guard ($uri === ''). + $xml = <<<'XML' + + + t + +XML; + // Attempting to use the 'empty' prefix should fail because it wasn't registered. + libxml_use_internal_errors(true); + try { + $doc = new DOMDocument(); + $doc->loadXML($xml); + $context = $doc->getElementsByTagName('child')->item(0); + $this->assertInstanceOf(\DOMElement::class, $context); + + $xp = XPath::getXPath($context); + $this->expectException(\Throwable::class); + // The XPath helper wraps libxml errors into a generic message: + $this->expectExceptionMessage('Malformed XPath query or invalid contextNode provided.'); + XPath::xpQuery($context, 'empty:whatever', $xp); + } finally { + $errors = libxml_get_errors(); + $this->assertEquals("xmlns:empty: Empty XML namespace is not allowed\n", $errors[0]->message); + $this->assertEquals("Undefined namespace prefix\n", $errors[1]->message); + libxml_clear_errors(); + libxml_use_internal_errors(false); + } + } + + + public function testXmlnsPrefixedDeclarationRegistersNamespaceViaAttributeBranch(): void + { + // Build DOM programmatically to ensure xmlns:foo exists as attribute. + $doc = new DOMDocument('1.0', 'UTF-8'); + $root = $doc->createElement('root'); + $doc->appendChild($root); + + // Add xmlns:foo on the root (attribute-branch should detect and register) + $root->setAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:foo', 'https://example.org/foo'); + + // Deep subtree that uses foo prefix but the context node itself is unprefixed + $ctx = $doc->createElement('ctx'); + $root->appendChild($ctx); + + $fooItem = $doc->createElementNS('https://example.org/foo', 'foo:item', 'ok'); + $ctx->appendChild($fooItem); + + // Use the unprefixed context to ensure ancestor-walk is required. + $xp = XPath::getXPath($ctx); + $nodes = XPath::xpQuery($ctx, 'foo:item', $xp); + + // If attribute-branch registered 'foo', the query resolves. + $this->assertCount(1, $nodes); + $this->assertSame('ok', $nodes[0]->textContent); + } + + /** + * Provides relative file paths for the two XML variants. + * + * @return array + */ + public static function xmlVariantsProviderForTopLevelSlatePerson(): array + { + $base = dirname(__FILE__, 3) . '/tests/resources/xml'; + + return [ + "Register Subtree Prefixes" => [$base . '/success_response_a.xml'], + "Register Ancestor Namespaces" => [$base . '/success_response_b.xml'], + ]; + } + + /** + * Ensure that absolute XPath '/foo:serviceResponse/foo:authenticationSuccess/slate:person' + * finds the same top-level slate:person regardless of whether it appears before or after + * cas:attributes in the document, even when the slate prefix is only declared on the element itself. + */ + #[DataProvider('xmlVariantsProviderForTopLevelSlatePerson')] + public function testAbsoluteXPathFindsTopLevelSlatePerson(string $filePath): void + { + $doc = DOMDocumentFactory::fromFile($filePath); + + $fooNs = 'https://example.org/foo'; + /** @var \DOMElement|null $authn */ + $authn = $doc->getElementsByTagNameNS($fooNs, 'authenticationSuccess')->item(0); + $this->assertNotNull($authn, 'authenticationSuccess element not found'); + + $xp = XPath::getXPath($authn); + $query = '/foo:serviceResponse/foo:authenticationSuccess/slate:person'; + + $nodes = XPath::xpQuery($authn, $query, $xp); + + $this->assertSame(1, count($nodes), 'Expected exactly one top-level slate:person'); + $this->assertSame('12345_top', trim($nodes[0]->textContent)); + } } diff --git a/tests/resources/xml/success_response_a.xml b/tests/resources/xml/success_response_a.xml new file mode 100644 index 00000000..ab1c3d85 --- /dev/null +++ b/tests/resources/xml/success_response_a.xml @@ -0,0 +1,21 @@ + + + jdoe + + 2025-11-07T22:00:24+02:00 + true + true + Doe + John + jdoe@example.edu + jdoe@example.edu + + 12345 + Fall-2025 + ABC-123 + + + 12345_top + + \ No newline at end of file diff --git a/tests/resources/xml/success_response_b.xml b/tests/resources/xml/success_response_b.xml new file mode 100644 index 00000000..855df978 --- /dev/null +++ b/tests/resources/xml/success_response_b.xml @@ -0,0 +1,20 @@ + + + jdoe + 12345_top + + 2025-11-07T22:00:24+02:00 + true + true + Doe + John + jdoe@example.edu + jdoe@example.edu + + 12345 + Fall-2025 + ABC-123 + + + \ No newline at end of file From 66bbb302de3827bcba8e3fe97ba815c21ffa66f5 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Fri, 14 Nov 2025 12:27:03 +0200 Subject: [PATCH 7/7] Fix quality errors --- tests/XPath/XPathTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/XPath/XPathTest.php b/tests/XPath/XPathTest.php index 11f0aa1c..dbec0617 100644 --- a/tests/XPath/XPathTest.php +++ b/tests/XPath/XPathTest.php @@ -294,10 +294,11 @@ public function testXmlnsPrefixedDeclarationRegistersNamespaceViaAttributeBranch $this->assertSame('ok', $nodes[0]->textContent); } + /** * Provides relative file paths for the two XML variants. * - * @return array + * @return array */ public static function xmlVariantsProviderForTopLevelSlatePerson(): array {