diff --git a/src/XPath/XPath.php b/src/XPath/XPath.php index c4d47955..9fbacbd1 100644 --- a/src/XPath/XPath.php +++ b/src/XPath/XPath.php @@ -20,6 +20,29 @@ */ class XPath { + /** + * Search for an element with a certain name among the children of a reference element. + * + * @param \DOMNode $ref The DOMDocument or DOMElement where encrypted data is expected to be found as a child. + * @param string $name The name (possibly prefixed) of the element we are looking for. + * + * @return \DOMElement|false The element we are looking for, or false when not found. + * + * @throws \RuntimeException If no DOM document is available. + */ + public static function findElement(DOMNode $ref, string $name): DOMElement|false + { + $doc = $ref instanceof DOMDocument ? $ref : $ref->ownerDocument; + if ($doc === null) { + throw new RuntimeException('Cannot search, no DOMDocument available'); + } + + $nodeset = self::getXPath($doc)->query('./' . $name, $ref); + + return $nodeset->item(0) ?? false; + } + + /** * Get an instance of DOMXPath associated with a DOMNode * @@ -30,9 +53,10 @@ class XPath * custom prefixes declared anywhere up the tree can be used in queries. * * @param \DOMNode $node The associated node + * @param bool $autoregister Whether to auto-register all namespaces used in the document * @return \DOMXPath */ - public static function getXPath(DOMNode $node): DOMXPath + public static function getXPath(DOMNode $node, bool $autoregister = false): DOMXPath { static $xpCache = null; @@ -53,8 +77,10 @@ public static function getXPath(DOMNode $node): DOMXPath // Enrich with ancestor-declared prefixes for this document context. $prefixToUri = self::registerAncestorNamespaces($xpCache, $node); - // Single, bounded subtree scan to pick up descendant-only declarations. - self::registerSubtreePrefixes($xpCache, $node, $prefixToUri); + if ($autoregister) { + // Single, bounded subtree scan to pick up descendant-only declarations. + self::registerSubtreePrefixes($xpCache, $node, $prefixToUri); + } return $xpCache; } @@ -142,19 +168,35 @@ private static function registerSubtreePrefixes(DOMXPath $xp, DOMNode $node, arr return; } - $visited = 0; +// $visited = 0; - /** @var array<\DOMElement> $queue */ - $queue = [$root]; + /** @var array $queue */ + $queue = [[$root, 0]]; while ($queue) { /** @var \DOMElement $el */ - $el = array_shift($queue); - - if (++$visited > C_XML::UNBOUNDED_LIMIT) { - throw new \RuntimeException(__METHOD__ . ': exceeded subtree traversal limit'); + /** @var int $depth */ + [$el, $depth] = array_shift($queue); + + // Depth guard: cap traversal at UNBOUNDED_LIMIT (root = depth 0). + // Breaking here halts further descent to avoid pathological depth and excessive work, + // which is safer in production than risking runaway traversal or hard failures. + // Trade-off: deeper descendant-only prefixes may remain unregistered, so some + // prefixed XPath queries might fail; overall processing continues gracefully. + if ($depth >= C_XML::UNBOUNDED_LIMIT) { + break; } +// if (++$visited > C_XML::UNBOUNDED_LIMIT) { +// // Safety valve: stop further traversal to avoid unbounded work and noisy exceptions. +// // Returning here halts namespace registration for this subtree, which is safer in +// // production than risking pathological O(n) behavior or a hard failure (e.g. throwing +// // \RuntimeException(__METHOD__ . ': exceeded subtree traversal limit')). +// // Trade-off: some descendant-only prefixes may remain unregistered, so related XPath +// // queries might fail, but overall processing continues gracefully. +// break; +// } + // Element prefix if ($el->prefix && !isset($prefixToUri[$el->prefix])) { $uri = $el->namespaceURI; @@ -189,7 +231,7 @@ private static function registerSubtreePrefixes(DOMXPath $xp, DOMNode $node, arr // Enqueue children (only DOMElement to keep types precise) foreach ($el->childNodes as $child) { if ($child instanceof DOMElement) { - $queue[] = $child; + $queue[] = [$child, $depth + 1]; } } } diff --git a/tests/XML/SchemaValidatableElementTraitTest.php b/tests/XML/SchemaValidatableElementTraitTest.php index a7d6877a..ed147574 100644 --- a/tests/XML/SchemaValidatableElementTraitTest.php +++ b/tests/XML/SchemaValidatableElementTraitTest.php @@ -4,7 +4,7 @@ namespace SimpleSAML\Test\XML; -use DOMDocument; +use PHPUnit\Framework\Attributes\DoesNotPerformAssertions; use PHPUnit\Framework\TestCase; use SimpleSAML\Test\Helper\Base64BinaryElement; use SimpleSAML\Test\Helper\BooleanElement; @@ -20,13 +20,13 @@ */ final class SchemaValidatableElementTraitTest extends TestCase { + #[DoesNotPerformAssertions] public function testSchemaValidationPasses(): void { $file = 'tests/resources/xml/ssp_StringElement.xml'; $chunk = DOMDocumentFactory::fromFile($file); $document = StringElement::schemaValidate($chunk); - $this->assertInstanceOf(DOMDocument::class, $document); } diff --git a/tests/XML/TypedTextContentTraitTest.php b/tests/XML/TypedTextContentTraitTest.php index 316fc186..19ec0764 100644 --- a/tests/XML/TypedTextContentTraitTest.php +++ b/tests/XML/TypedTextContentTraitTest.php @@ -4,6 +4,7 @@ namespace SimpleSAML\Test\XML; +use PHPUnit\Framework\Attributes\DoesNotPerformAssertions; use PHPUnit\Framework\TestCase; use SimpleSAML\Test\Helper\Base64BinaryElement; use SimpleSAML\Test\Helper\BooleanElement; @@ -19,6 +20,7 @@ */ final class TypedTextContentTraitTest extends TestCase { + #[DoesNotPerformAssertions] public function testTypedContentPassesForString(): void { $file = 'tests/resources/xml/ssp_StringElement.xml'; @@ -27,10 +29,10 @@ public function testTypedContentPassesForString(): void $elt = $doc->documentElement; $stringElt = StringElement::fromXML($elt); - $this->assertInstanceOf(StringElement::class, $stringElt); } + #[DoesNotPerformAssertions] public function testTypedContentPassesForBoolean(): void { $file = 'tests/resources/xml/ssp_BooleanElement.xml'; @@ -39,7 +41,6 @@ public function testTypedContentPassesForBoolean(): void $elt = $doc->documentElement; $stringElt = BooleanElement::fromXML($elt); - $this->assertInstanceOf(BooleanElement::class, $stringElt); } diff --git a/tests/XPath/XPathTest.php b/tests/XPath/XPathTest.php index dbec0617..21875b68 100644 --- a/tests/XPath/XPathTest.php +++ b/tests/XPath/XPathTest.php @@ -186,7 +186,7 @@ public function testNormalizationFromAttributeNode(): void $this->assertInstanceOf(\DOMElement::class, $elt); $attr = $elt->getAttributeNodeNS('urn:bar', 'attr'); - $this->assertInstanceOf(\DOMAttr::class, $attr); + /** @var \DOMAttr $attr */ // getXPath should normalize from DOMAttr to the element and ensure 'bar' is registered. $xp = XPath::getXPath($attr); @@ -305,8 +305,26 @@ 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'], + "Ancestor-declared 'slate'; top-level person AFTER attributes" => [ + $base . '/success_response_a.xml', + false, + false, + ], + "Ancestor-declared 'slate'; top-level person BEFORE attributes" => [ + $base . '/success_response_b.xml', + false, + false, + ], + "Descendant-only 'slate'; no ancestor binding (fails without autoregister)" => [ + $base . '/success_response_c.xml', + false, + true, + ], + "Descendant-only 'slate'; no ancestor binding (succeeds with autoregister)" => [ + $base . '/success_response_c.xml', + true, + false, + ], ]; } @@ -316,21 +334,102 @@ public static function xmlVariantsProviderForTopLevelSlatePerson(): array * 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 - { + public function testAbsoluteXPathFindsTopLevelSlatePerson( + string $filePath, + bool $autoregister, + bool $shouldFail, + ): 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'); + /** @var \DOMElement|null $attributesNode */ + $attributesNode = $doc->getElementsByTagNameNS($fooNs, 'attributes')->item(0); + $this->assertNotNull($attributesNode, 'Attributes element not found'); - $xp = XPath::getXPath($authn); + $xp = XPath::getXPath($attributesNode, $autoregister); $query = '/foo:serviceResponse/foo:authenticationSuccess/slate:person'; - $nodes = XPath::xpQuery($authn, $query, $xp); + if ($shouldFail) { + libxml_use_internal_errors(true); + try { + $this->expectException(\SimpleSAML\Assert\AssertionFailedException::class); + $this->expectExceptionMessage('Malformed XPath query or invalid contextNode provided.'); + XPath::xpQuery($attributesNode, $query, $xp); + } finally { + $errors = libxml_get_errors(); + $this->assertNotEmpty($errors); + $this->assertSame("Undefined namespace prefix\n", $errors[0]->message); + libxml_clear_errors(); + libxml_use_internal_errors(false); + } + return; + } - $this->assertSame(1, count($nodes), 'Expected exactly one top-level slate:person'); + $nodes = XPath::xpQuery($attributesNode, $query, $xp); + $this->assertCount(1, $nodes); $this->assertSame('12345_top', trim($nodes[0]->textContent)); } + + + public function testFindElementFindsDirectChildUnprefixed(): void + { + $doc = new DOMDocument(); + $doc->loadXML('t'); + + $root = $doc->documentElement; + $this->assertInstanceOf(DOMElement::class, $root); + + $found = XPath::findElement($root, 'target'); + $this->assertInstanceOf(DOMElement::class, $found); + $this->assertSame('target', $found->localName); + $this->assertSame('t', $found->textContent); + } + + + public function testFindElementFindsDirectChildWithPrefixWhenNsOnRoot(): void + { + $xml = <<<'XML' + + + ok + +XML; + $doc = new DOMDocument(); + $doc->loadXML($xml); + + $root = $doc->documentElement; + $this->assertInstanceOf(DOMElement::class, $root); + + // Namespace is declared on root, so getXPath($doc) used by findElement knows 'foo' + $found = XPath::findElement($root, 'foo:item'); + $this->assertInstanceOf(DOMElement::class, $found); + $this->assertSame('item', $found->localName); + $this->assertSame('https://example.org/foo', $found->namespaceURI); + $this->assertSame('ok', $found->textContent); + } + + + public function testFindElementReturnsFalseWhenNotFoundAndDoesNotDescend(): void + { + // 'target' is a grandchild; findElement should only match direct children via './name' + $doc = new DOMDocument(); + $doc->loadXML(''); + + $root = $doc->documentElement; + $this->assertInstanceOf(DOMElement::class, $root); + + $found = XPath::findElement($root, 'target'); + $this->assertFalse($found, 'Should return false for non-direct child'); + } + + + public function testFindElementThrowsIfNoOwnerDocument(): void + { + // A standalone DOMElement (not created by a DOMDocument) has no ownerDocument + $ref = new \DOMElement('container'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Cannot search, no DOMDocument available'); + XPath::findElement($ref, 'anything'); + } } diff --git a/tests/resources/xml/success_response_c.xml b/tests/resources/xml/success_response_c.xml new file mode 100644 index 00000000..a8a756fb --- /dev/null +++ b/tests/resources/xml/success_response_c.xml @@ -0,0 +1,23 @@ + + + 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