Skip to content

Commit a71e04b

Browse files
ioigoumeioigoume
committed
xpath ancestor registration
1 parent f4133d8 commit a71e04b

File tree

5 files changed

+191
-7
lines changed

5 files changed

+191
-7
lines changed

‎composer.json‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@
4242
"ext-pcre": "*",
4343
"ext-spl": "*",
4444

45-
"simplesamlphp/assert": "~1.9.0",
46-
"simplesamlphp/composer-xmlprovider-installer": "~1.1.0"
45+
"simplesamlphp/assert": "~1.9.0"
4746
},
4847
"require-dev": {
4948
"simplesamlphp/simplesamlphp-test-framework": "~1.10.1"
@@ -56,8 +55,23 @@
5655
"allow-plugins": {
5756
"composer/package-versions-deprecated": true,
5857
"dealerdirect/phpcodesniffer-composer-installer": true,
59-
"phpstan/extension-installer": true,
60-
"simplesamlphp/composer-xmlprovider-installer": true
58+
"phpstan/extension-installer": true
6159
}
60+
},
61+
"scripts": {
62+
"pre-commit": [
63+
"vendor/bin/phpcs -p",
64+
"vendor/bin/composer-require-checker check --config-file=tools/composer-require-checker.json composer.json",
65+
"vendor/bin/phpstan analyze -c phpstan.neon",
66+
"vendor/bin/phpstan analyze -c phpstan-dev.neon",
67+
"vendor/bin/composer-unused",
68+
"vendor/bin/phpunit --no-coverage --testdox"
69+
],
70+
"tests": [
71+
"vendor/bin/phpunit --no-coverage"
72+
],
73+
"propose-fix": [
74+
"vendor/bin/phpcs --report=diff"
75+
]
6276
}
6377
}

‎src/XPath/XPath.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace SimpleSAML\XPath;
66

77
use DOMDocument;
8+
use DOMElement;
89
use DOMNode;
910
use DOMXPath;
1011
use SimpleSAML\XML\Assert\Assert;
@@ -21,6 +22,12 @@ class XPath
2122
/**
2223
* Get an instance of DOMXPath associated with a DOMNode
2324
*
25+
* - Reuses a cached DOMXPath per document.
26+
* - Registers core XML-related namespaces: 'xml' and 'xs'.
27+
* - Enriches the XPath with all prefixed xmlns declarations found on the
28+
* current node and its ancestors (up to the document element), so
29+
* custom prefixes declared anywhere up the tree can be used in queries.
30+
*
2431
* @param \DOMNode $node The associated node
2532
* @return \DOMXPath
2633
*/
@@ -42,10 +49,68 @@ public static function getXPath(DOMNode $node): DOMXPath
4249
$xpCache->registerNamespace('xml', C_XML::NS_XML);
4350
$xpCache->registerNamespace('xs', C_XS::NS_XS);
4451

52+
// Enrich with ancestor-declared prefixes for this document context.
53+
self::registerAncestorNamespaces($xpCache, $node);
54+
4555
return $xpCache;
4656
}
4757

4858

59+
/**
60+
* Walk from the given node up to the document element, registering all prefixed xmlns declarations.
61+
*
62+
* Safety:
63+
* - Only attributes in the XMLNS namespace (http://www.w3.org/2000/xmlns/).
64+
* - Skip default xmlns (localName === 'xmlns') because XPath requires prefixes.
65+
* - Skip empty URIs.
66+
* - Do not override core 'xml' and 'xs' prefixes (already bound).
67+
* - Nearest binding wins during this pass (prefixes are added once).
68+
*
69+
* @param \DOMXPath $xp
70+
* @param \DOMNode $node
71+
*/
72+
private static function registerAncestorNamespaces(DOMXPath $xp, DOMNode $node): void
73+
{
74+
$xmlnsNs = 'http://www.w3.org/2000/xmlns/';
75+
76+
// Avoid re-binding while walking upwards.
77+
$registered = [
78+
'xml' => true,
79+
'xs' => true,
80+
];
81+
82+
// Start from the nearest element (or documentElement if a DOMDocument is passed).
83+
$current = $node instanceof DOMDocument
84+
? $node->documentElement
85+
: ($node instanceof DOMElement ? $node : $node->parentNode);
86+
87+
while ($current instanceof DOMElement) {
88+
if ($current->hasAttributes()) {
89+
foreach ($current->attributes as $attr) {
90+
if ($attr->namespaceURI !== $xmlnsNs) {
91+
continue;
92+
}
93+
$prefix = $attr->localName; // e.g., 'slate' for xmlns:slate, 'xmlns' for default
94+
$uri = (string) $attr->nodeValue;
95+
96+
if (
97+
$prefix === null || $prefix === '' ||
98+
$prefix === 'xmlns' || $uri === '' ||
99+
isset($registered[$prefix])
100+
) {
101+
continue;
102+
}
103+
104+
$xp->registerNamespace($prefix, $uri);
105+
$registered[$prefix] = true;
106+
}
107+
}
108+
109+
$current = $current->parentNode instanceof DOMElement ? $current->parentNode : null;
110+
}
111+
}
112+
113+
49114
/**
50115
* Do an XPath query on an XML node.
51116
*

‎tests/XML/ExtendableAttributesTraitTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ public function getAttributeNamespace(): array|string
8888
public function testEmptyNamespaceArrayThrowsAnException(): void
8989
{
9090
$this->expectException(AssertionFailedException::class);
91-
// @phpstan-ignore expr.resultUnused
9291
new class ([]) extends ExtendableAttributesElement {
9392
/**
9493
* @return array<int, string>|string

‎tests/XML/ExtendableElementTraitTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ public static function setUpBeforeClass(): void
8585
public function testIllegalNamespaceComboThrowsAnException(): void
8686
{
8787
$this->expectException(AssertionFailedException::class);
88-
// @phpstan-ignore expr.resultUnused
8988
new class ([]) extends ExtendableElement {
9089
/**
9190
* @return array<int, string>|string
@@ -104,7 +103,6 @@ public function getElementNamespace(): array|string
104103
public function testEmptyNamespaceArrayThrowsAnException(): void
105104
{
106105
$this->expectException(AssertionFailedException::class);
107-
// @phpstan-ignore expr.resultUnused
108106
new class ([]) extends ExtendableElement {
109107
/**
110108
* @return array<int, string>|string

‎tests/XPath/XPathTest.php‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\XPath;
6+
7+
use PHPUnit\Framework\Attributes\CoversClass;
8+
use PHPUnit\Framework\TestCase;
9+
use SimpleSAML\XPath\XPath;
10+
11+
/**
12+
* Tests for the SimpleSAML\XPath\XPath helper.
13+
*/
14+
#[CoversClass(XPath::class)]
15+
final class XPathTest extends TestCase
16+
{
17+
public function testGetXPathCachesPerDocumentAndRegistersCoreNamespaces(): void
18+
{
19+
// Doc A with an xml:space attribute to validate 'xml' prefix usage works
20+
$docA = new \DOMDocument();
21+
$docA->loadXML(<<<'XML'
22+
<?xml version="1.0" encoding="UTF-8"?>
23+
<root xml:space="preserve" xmlns:xml="http://www.w3.org/XML/1998/namespace">
24+
<child>value</child>
25+
</root>
26+
XML);
27+
28+
// Doc B is different
29+
$docB = new \DOMDocument();
30+
$docB->loadXML(<<<'XML'
31+
<?xml version="1.0" encoding="UTF-8"?>
32+
<another><node/></another>
33+
XML);
34+
35+
$xpA1 = XPath::getXPath($docA);
36+
$xpA2 = XPath::getXPath($docA);
37+
$xpB = XPath::getXPath($docB);
38+
39+
// Cached instance reused per same document
40+
$this->assertSame($xpA1, $xpA2);
41+
// Different document => different DOMXPath instance
42+
$this->assertNotSame($xpA1, $xpB);
43+
44+
// 'xml' prefix registered: query should be valid and return xml:space attribute
45+
$rootA = $docA->documentElement;
46+
$this->assertInstanceOf(\DOMElement::class, $rootA);
47+
$attrs = XPath::xpQuery($rootA, '@xml:space', $xpA1);
48+
$this->assertCount(1, $attrs);
49+
$this->assertSame('preserve', $attrs[0]->nodeValue);
50+
}
51+
52+
53+
public function testAncestorNamespaceRegistrationAllowsCustomPrefixes(): void
54+
{
55+
// Custom namespace declared on the root; query from a descendant node
56+
$xml = <<<'XML'
57+
<?xml version="1.0" encoding="UTF-8"?>
58+
<r xmlns:foo="https://example.org/foo">
59+
<a>
60+
<b>
61+
<foo:item>ok</foo:item>
62+
</b>
63+
</a>
64+
</r>
65+
XML;
66+
$doc = new \DOMDocument();
67+
$doc->loadXML($xml);
68+
69+
// Use a deep context node to ensure ancestor-walk picks up xmlns:foo from root
70+
$context = $doc->getElementsByTagName('b')->item(0);
71+
$this->assertInstanceOf(\DOMElement::class, $context);
72+
73+
$xp = XPath::getXPath($context);
74+
75+
$nodes = XPath::xpQuery($context, 'foo:item', $xp);
76+
$this->assertCount(1, $nodes);
77+
$this->assertSame('ok', $nodes[0]->textContent);
78+
}
79+
80+
81+
public function testXpQueryThrowsOnMalformedExpression(): void
82+
{
83+
$doc = new \DOMDocument();
84+
$doc->loadXML('<root><x/></root>');
85+
$xp = XPath::getXPath($doc);
86+
87+
// If xpQuery throws a specific exception, put that class here instead of \Throwable.
88+
$this->expectException(\Throwable::class);
89+
// Keep message assertion resilient to libxml version differences.
90+
$this->expectExceptionMessageMatches('/(XPath|expression).*invalid|malformed|error/i');
91+
92+
// Malformed XPath: missing closing bracket
93+
$root = $doc->documentElement;
94+
$this->assertInstanceOf(\DOMElement::class, $root);
95+
96+
// Avoid emitting a PHP warning; let xpQuery surface it as an exception.
97+
\libxml_use_internal_errors(true);
98+
try {
99+
XPath::xpQuery($root, '//*[', $xp);
100+
} finally {
101+
$errors = \libxml_get_errors();
102+
self::assertCount(1, $errors);
103+
self::assertEquals("Invalid expression\n", $errors[0]->message);
104+
\libxml_clear_errors();
105+
\libxml_use_internal_errors(false);
106+
}
107+
}
108+
}

0 commit comments

Comments
 (0)