Skip to content

Commit 1925855

Browse files
nielsdoscmb69
authored andcommitted
Fix bug 69168: DomNode::getNodePath() returns invalid path
Upon freeing libxslt's context, every document which is not the *main* document will be freed by libxslt. If a node of a document which is not the main document gets returned to userland, we'd free the node twice: - first by the cleanup of the xslt context - and then by our own refcounting mechanism. This was reported in bug 49634, and was fixed by always copying the node (and later re-fixed in bug 70078). The original fix is not entirely correct unfortunately because of the following two main reasons: - modifications to the node will only modify the copy, and not the original - accesses to the parent, path, ... will not work This patch fixes it properly by only copying the node if it origins from a document other than the main document. Co-authored-by: juha.ikavalko@agentit.fi Closes GH-10318.
1 parent 3030d95 commit 1925855

File tree

3 files changed

+59
-1
lines changed

3 files changed

+59
-1
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,7 @@ PHP NEWS
9898
. Fixed bug #51056: blocking fread() will block even if data is available.
9999
(Jakub Zelenka)
100100

101+
- XSLTProcessor:
102+
. Fixed bug #69168 (DomNode::getNodePath() returns invalid path). (nielsdos)
103+
101104
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

ext/xsl/tests/bug69168.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
bug #69168 (DomNode::getNodePath() returns invalid path)
3+
--EXTENSIONS--
4+
xsl
5+
--FILE--
6+
<?php
7+
$xml = <<<EOB
8+
<allusers><user><uid>bob</uid></user><user><uid>joe</uid></user></allusers>
9+
EOB;
10+
$xsl = <<<EOB
11+
<?xml version="1.0" encoding="UTF-8"?>
12+
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:php="http://php.net/xsl">
13+
<xsl:template match="allusers">
14+
<xsl:for-each select="user">
15+
<xsl:value-of select="php:function('getPath',uid)"/><br />
16+
</xsl:for-each>
17+
</xsl:template>
18+
</xsl:stylesheet>
19+
EOB;
20+
21+
function getPath($input){
22+
$input[0]->nodeValue .= 'a';
23+
return $input[0]->getNodePath() . ' = ' . $input[0]->nodeValue;
24+
}
25+
26+
$proc = new XSLTProcessor();
27+
$proc->registerPHPFunctions();
28+
$xslDoc = new DOMDocument();
29+
$xslDoc->loadXML($xsl);
30+
@$proc->importStyleSheet($xslDoc);
31+
$xmlDoc = new DOMDocument();
32+
$xmlDoc->loadXML($xml);
33+
echo @$proc->transformToXML($xmlDoc);
34+
35+
// Tests modification of the nodes
36+
var_dump($xmlDoc->firstChild->firstChild->firstChild->getNodePath());
37+
var_dump($xmlDoc->firstChild->firstChild->firstChild->nodeValue);
38+
?>
39+
--EXPECT--
40+
<?xml version="1.0"?>
41+
/allusers/user[1]/uid = boba<br/>/allusers/user[2]/uid = joea<br/>
42+
string(21) "/allusers/user[1]/uid"
43+
string(4) "boba"

ext/xsl/xsltprocessor.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,19 @@ static void xsl_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, int t
194194
node->parent = nsparent;
195195
node->ns = curns;
196196
} else {
197-
node = xmlDocCopyNode(node, domintern->document->ptr, 1);
197+
/**
198+
* Upon freeing libxslt's context, every document which is not the *main* document will be freed by libxslt.
199+
* If a node of a document which is *not the main* document gets returned to userland, we'd free the node twice:
200+
* first by the cleanup of the xslt context, and then by our own refcounting mechanism.
201+
* To prevent this, we'll take a copy if the node is not from the main document.
202+
* It is important that we do not copy the node unconditionally, because that means that:
203+
* - modifications to the node will only modify the copy, and not the original
204+
* - accesses to the parent, path, ... will not work
205+
*/
206+
xsltTransformContextPtr transform_ctxt = (xsltTransformContextPtr) ctxt->context->extra;
207+
if (node->doc != transform_ctxt->document->doc) {
208+
node = xmlDocCopyNode(node, domintern->document->ptr, 1);
209+
}
198210
}
199211

200212
php_dom_create_object(node, &child, domintern);

0 commit comments

Comments
 (0)