Skip to content

Commit 3e33eda

Browse files
nielsdosnielsdos
committed
Fix cloning attribute with namespace disappearing namespace
Closes GH-12547.
1 parent a470c4a commit 3e33eda

10 files changed

+184
-0
lines changed

‎NEWS

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ DOM:
99
. Added DOMNode::compareDocumentPosition(). (nielsdos)
1010
. Implement #53655 (Improve speed of DOMNode::C14N() on large XML documents).
1111
(nielsdos)
12+
. Fix cloning attribute with namespace disappearing namespace. (nielsdos)
1213

1314
Intl:
1415
. Added IntlDateFormatter::PATTERN constant. (David Carlier)

‎ext/dom/element.c

+2
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,7 @@ PHP_METHOD(DOMElement, setAttributeNode)
640640
}
641641

642642
xmlAddChild(nodep, (xmlNodePtr) attrp);
643+
php_dom_reconcile_attribute_namespace_after_insertion(attrp);
643644

644645
/* Returns old property if removed otherwise NULL */
645646
if (existattrp != NULL) {
@@ -1012,6 +1013,7 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
10121013
}
10131014

10141015
xmlAddChild(nodep, (xmlNodePtr) attrp);
1016+
php_dom_reconcile_attribute_namespace_after_insertion(attrp);
10151017

10161018
/* Returns old property if removed otherwise NULL */
10171019
if (existattrp != NULL) {

‎ext/dom/node.c

+8
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,7 @@ PHP_METHOD(DOMNode, appendChild)
12591259
if (UNEXPECTED(new_child == NULL)) {
12601260
goto cannot_add;
12611261
}
1262+
php_dom_reconcile_attribute_namespace_after_insertion((xmlAttrPtr) new_child);
12621263
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
12631264
xmlNodePtr last = child->last;
12641265
new_child = _php_dom_insert_fragment(nodep, nodep->last, NULL, child, intern, childobj);
@@ -1362,6 +1363,13 @@ PHP_METHOD(DOMNode, cloneNode)
13621363
}
13631364
}
13641365

1366+
if (node->type == XML_ATTRIBUTE_NODE && n->ns != NULL && node->ns == NULL) {
1367+
/* Let reconciliation deal with this. The lifetime of the namespace poses no problem
1368+
* because we're increasing the refcount of the document proxy at the return.
1369+
* libxml2 doesn't set the ns because it can't know that this is safe. */
1370+
node->ns = n->ns;
1371+
}
1372+
13651373
/* If document cloned we want a new document proxy */
13661374
if (node->doc != n->doc) {
13671375
intern = NULL;

‎ext/dom/php_dom.c

+18
Original file line numberDiff line numberDiff line change
@@ -1458,6 +1458,22 @@ static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePt
14581458
}
14591459
}
14601460

1461+
void php_dom_reconcile_attribute_namespace_after_insertion(xmlAttrPtr attrp)
1462+
{
1463+
ZEND_ASSERT(attrp != NULL);
1464+
1465+
if (attrp->ns != NULL) {
1466+
/* Try to link to an existing namespace. If that won't work, reconcile. */
1467+
xmlNodePtr nodep = attrp->parent;
1468+
xmlNsPtr matching_ns = xmlSearchNs(nodep->doc, nodep, attrp->ns->prefix);
1469+
if (matching_ns && xmlStrEqual(matching_ns->href, attrp->ns->href)) {
1470+
attrp->ns = matching_ns;
1471+
} else {
1472+
xmlReconciliateNs(nodep->doc, nodep);
1473+
}
1474+
}
1475+
}
1476+
14611477
static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep)
14621478
{
14631479
/* Ideally we'd use the DOM-wrapped version, but we cannot: https://github.com/php/php-src/pull/12308. */
@@ -1474,6 +1490,8 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep
14741490

14751491
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
14761492
{
1493+
ZEND_ASSERT(nodep->type != XML_ATTRIBUTE_NODE);
1494+
14771495
/* Although the node type will be checked by the libxml2 API,
14781496
* we still want to do the internal reconciliation conditionally. */
14791497
if (nodep->type == XML_ELEMENT_NODE) {

‎ext/dom/php_dom.h

+1
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ zend_string *dom_node_get_node_name_attribute_or_element(const xmlNode *nodep);
152152
bool php_dom_is_node_connected(const xmlNode *node);
153153
bool php_dom_adopt_node(xmlNodePtr nodep, dom_object *dom_object_new_document, xmlDocPtr new_document);
154154
xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri);
155+
void php_dom_reconcile_attribute_namespace_after_insertion(xmlAttrPtr attrp);
155156

156157
/* parentnode */
157158
void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc);

‎ext/dom/tests/DOMDocument_importNode_attribute_prefix_conflict.phpt

+6
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ $dom1->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:b
2626
$dom2->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" xmlns:foo="http://php.net/2"/>');
2727
$attribute = $dom1->documentElement->getAttributeNode('foo:bar');
2828
$imported = $dom2->importNode($attribute);
29+
var_dump($imported->prefix, $imported->namespaceURI);
2930
$dom2->documentElement->setAttributeNodeNS($imported);
31+
var_dump($imported->prefix, $imported->namespaceURI);
3032

3133
echo $dom1->saveXML();
3234
echo $dom2->saveXML();
@@ -54,6 +56,10 @@ echo $dom2->saveXML();
5456
<?xml version="1.0"?>
5557
<container xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
5658
--- Non-default namespace test case with a default namespace in the destination ---
59+
string(7) "default"
60+
string(14) "http://php.net"
61+
string(7) "default"
62+
string(14) "http://php.net"
5763
<?xml version="1.0"?>
5864
<container xmlns:foo="http://php.net" foo:bar="yes"/>
5965
<?xml version="1.0"?>

‎ext/dom/tests/clone_attribute_namespace_01.phpt

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
--TEST--
2+
Cloning an attribute should retain its namespace 01
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function createTestDocument() {
9+
$dom = new DOMDocument;
10+
$dom->loadXML('<?xml version="1.0"?><container/>');
11+
$dom->documentElement->setAttributeNs("some:ns", "foo:bar", "value");
12+
13+
$attr = $dom->documentElement->getAttributeNodeNs("some:ns", "bar");
14+
$clone = $attr->cloneNode(true);
15+
16+
return [$dom, $clone];
17+
}
18+
19+
[$dom, $clone] = createTestDocument();
20+
var_dump($clone->prefix, $clone->namespaceURI);
21+
22+
echo "--- Re-adding a namespaced attribute ---\n";
23+
24+
[$dom, $clone] = createTestDocument();
25+
$dom->documentElement->removeAttributeNs("some:ns", "bar");
26+
echo $dom->saveXML();
27+
$dom->documentElement->setAttributeNodeNs($clone);
28+
echo $dom->saveXML();
29+
30+
echo "--- Re-adding a namespaced attribute, with the namespace deleted (setAttributeNodeNs variation) ---\n";
31+
32+
function readd_test(string $method) {
33+
[$dom, $clone] = createTestDocument();
34+
$dom->documentElement->removeAttributeNs("some:ns", "bar");
35+
$dom->documentElement->removeAttribute("xmlns:foo");
36+
echo $dom->saveXML();
37+
$child = $dom->documentElement->appendChild($dom->createElement("child"));
38+
$child->{$method}($clone);
39+
echo $dom->saveXML();
40+
}
41+
42+
readd_test("setAttributeNodeNs");
43+
44+
echo "--- Re-adding a namespaced attribute, with the namespace deleted (setAttributeNode variation) ---\n";
45+
46+
readd_test("setAttributeNode");
47+
48+
echo "--- Re-adding a namespaced attribute, with the namespace deleted (appendChild variation) ---\n";
49+
50+
readd_test("appendChild");
51+
52+
echo "--- Removing the document reference should not crash ---\n";
53+
54+
[$dom, $clone] = createTestDocument();
55+
unset($dom);
56+
var_dump($clone->prefix, $clone->namespaceURI);
57+
58+
?>
59+
--EXPECT--
60+
string(3) "foo"
61+
string(7) "some:ns"
62+
--- Re-adding a namespaced attribute ---
63+
<?xml version="1.0"?>
64+
<container xmlns:foo="some:ns"/>
65+
<?xml version="1.0"?>
66+
<container xmlns:foo="some:ns" foo:bar="value"/>
67+
--- Re-adding a namespaced attribute, with the namespace deleted (setAttributeNodeNs variation) ---
68+
<?xml version="1.0"?>
69+
<container/>
70+
<?xml version="1.0"?>
71+
<container><child xmlns:foo="some:ns" foo:bar="value"/></container>
72+
--- Re-adding a namespaced attribute, with the namespace deleted (setAttributeNode variation) ---
73+
<?xml version="1.0"?>
74+
<container/>
75+
<?xml version="1.0"?>
76+
<container><child xmlns:foo="some:ns" foo:bar="value"/></container>
77+
--- Re-adding a namespaced attribute, with the namespace deleted (appendChild variation) ---
78+
<?xml version="1.0"?>
79+
<container/>
80+
<?xml version="1.0"?>
81+
<container><child xmlns:foo="some:ns" foo:bar="value"/></container>
82+
--- Removing the document reference should not crash ---
83+
string(3) "foo"
84+
string(7) "some:ns"

‎ext/dom/tests/clone_attribute_namespace_02.phpt

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
Cloning an attribute should retain its namespace 02
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = new DOMDocument;
9+
$dom->loadXML(<<<XML
10+
<?xml version="1.0"?>
11+
<container xmlns:foo="some:ns" foo:bar="1">
12+
<child xmlns:foo="some:other"/>
13+
</container>
14+
XML);
15+
16+
$clone = $dom->documentElement->getAttributeNodeNs("some:ns", "bar")->cloneNode(true);
17+
$dom->documentElement->firstElementChild->setAttributeNodeNs($clone);
18+
19+
echo $dom->saveXML();
20+
21+
?>
22+
--EXPECT--
23+
<?xml version="1.0"?>
24+
<container xmlns:foo="some:ns" foo:bar="1">
25+
<child xmlns:foo="some:other" xmlns:foo1="some:ns" foo1:bar="1"/>
26+
</container>

‎ext/dom/tests/import_attribute_namespace.phpt

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
Cloning an attribute should retain its namespace 02
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = new DOMDocument;
9+
$dom->loadXML(<<<XML
10+
<?xml version="1.0"?>
11+
<container xmlns:foo="some:ns" foo:bar="1"/>
12+
XML);
13+
14+
$dom2 = new DOMDocument;
15+
$dom2->loadXML(<<<XML
16+
<?xml version="1.0"?>
17+
<container xmlns:foo="some:other"/>
18+
XML);
19+
20+
$imported = $dom2->importNode($dom->documentElement->getAttributeNodeNs("some:ns", "bar"));
21+
var_dump($imported->prefix, $imported->namespaceURI);
22+
$dom2->documentElement->setAttributeNodeNs($imported);
23+
var_dump($imported->prefix, $imported->namespaceURI);
24+
25+
echo $dom->saveXML();
26+
echo $dom2->saveXML();
27+
28+
?>
29+
--EXPECT--
30+
string(7) "default"
31+
string(7) "some:ns"
32+
string(7) "default"
33+
string(7) "some:ns"
34+
<?xml version="1.0"?>
35+
<container xmlns:foo="some:ns" foo:bar="1"/>
36+
<?xml version="1.0"?>
37+
<container xmlns:foo="some:other" xmlns:default="some:ns" default:bar="1"/>

‎ext/phar/tests/gh12532.phpt

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
GH-12532 (PharData created from zip has incorrect timestamp)
33
--EXTENSIONS--
44
phar
5+
zlib
56
--FILE--
67
<?php
78

0 commit comments

Comments
 (0)