Skip to content

Commit b1d8e24

Browse files
nielsdosnielsdos
committed
Fix bug #67440: append_node of a DOMDocumentFragment does not reconcile namespaces
The test was amended from the original issue report. For the test: Co-authored-by: [email protected] The problem is that the regular dom_reconcile_ns() only works on a single node. We actually have to reconciliate the whole tree in case a fragment was added. This also required to move some code around such that this special case could be handled separately. Closes GH-11362.
1 parent 7812772 commit b1d8e24

File tree

6 files changed

+264
-49
lines changed

6 files changed

+264
-49
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ PHP NEWS
2121
(nielsdos)
2222
. Fixed bug GH-11347 (Memory leak when calling a static method inside an
2323
xpath query). (nielsdos)
24+
. Fixed bug #67440 (append_node of a DOMDocumentFragment does not reconcile
25+
namespaces). (nielsdos)
2426

2527
- Opcache:
2628
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)

‎ext/dom/node.c

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -943,12 +943,20 @@ PHP_METHOD(DOMNode, insertBefore)
943943
return;
944944
}
945945
}
946+
new_child = xmlAddPrevSibling(refp, child);
947+
if (UNEXPECTED(NULL == new_child)) {
948+
goto cannot_add;
949+
}
946950
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
951+
xmlNodePtr last = child->last;
947952
new_child = _php_dom_insert_fragment(parentp, refp->prev, refp, child, intern, childobj);
948-
}
949-
950-
if (new_child == NULL) {
953+
dom_reconcile_ns_list(parentp->doc, new_child, last);
954+
} else {
951955
new_child = xmlAddPrevSibling(refp, child);
956+
if (UNEXPECTED(NULL == new_child)) {
957+
goto cannot_add;
958+
}
959+
dom_reconcile_ns(parentp->doc, new_child);
952960
}
953961
} else {
954962
if (child->parent != NULL){
@@ -985,23 +993,28 @@ PHP_METHOD(DOMNode, insertBefore)
985993
return;
986994
}
987995
}
996+
new_child = xmlAddChild(parentp, child);
997+
if (UNEXPECTED(NULL == new_child)) {
998+
goto cannot_add;
999+
}
9881000
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
1001+
xmlNodePtr last = child->last;
9891002
new_child = _php_dom_insert_fragment(parentp, parentp->last, NULL, child, intern, childobj);
990-
}
991-
if (new_child == NULL) {
1003+
dom_reconcile_ns_list(parentp->doc, new_child, last);
1004+
} else {
9921005
new_child = xmlAddChild(parentp, child);
1006+
if (UNEXPECTED(NULL == new_child)) {
1007+
goto cannot_add;
1008+
}
1009+
dom_reconcile_ns(parentp->doc, new_child);
9931010
}
9941011
}
9951012

996-
if (NULL == new_child) {
997-
zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
998-
RETURN_THROWS();
999-
}
1000-
1001-
dom_reconcile_ns(parentp->doc, new_child);
1002-
10031013
DOM_RET_OBJ(new_child, &ret, intern);
1004-
1014+
return;
1015+
cannot_add:
1016+
zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
1017+
RETURN_THROWS();
10051018
}
10061019
/* }}} end dom_node_insert_before */
10071020

@@ -1066,9 +1079,10 @@ PHP_METHOD(DOMNode, replaceChild)
10661079

10671080
xmlUnlinkNode(oldchild);
10681081

1082+
xmlNodePtr last = newchild->last;
10691083
newchild = _php_dom_insert_fragment(nodep, prevsib, nextsib, newchild, intern, newchildobj);
10701084
if (newchild) {
1071-
dom_reconcile_ns(nodep->doc, newchild);
1085+
dom_reconcile_ns_list(nodep->doc, newchild, last);
10721086
}
10731087
} else if (oldchild != newchild) {
10741088
xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
@@ -1215,22 +1229,28 @@ PHP_METHOD(DOMNode, appendChild)
12151229
php_libxml_node_free_resource((xmlNodePtr) lastattr);
12161230
}
12171231
}
1232+
new_child = xmlAddChild(nodep, child);
1233+
if (UNEXPECTED(new_child == NULL)) {
1234+
goto cannot_add;
1235+
}
12181236
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
1237+
xmlNodePtr last = child->last;
12191238
new_child = _php_dom_insert_fragment(nodep, nodep->last, NULL, child, intern, childobj);
1220-
}
1221-
1222-
if (new_child == NULL) {
1239+
dom_reconcile_ns_list(nodep->doc, new_child, last);
1240+
} else {
12231241
new_child = xmlAddChild(nodep, child);
1224-
if (new_child == NULL) {
1225-
// TODO Convert to Error?
1226-
php_error_docref(NULL, E_WARNING, "Couldn't append node");
1227-
RETURN_FALSE;
1242+
if (UNEXPECTED(new_child == NULL)) {
1243+
goto cannot_add;
12281244
}
1245+
dom_reconcile_ns(nodep->doc, new_child);
12291246
}
12301247

1231-
dom_reconcile_ns(nodep->doc, new_child);
1232-
12331248
DOM_RET_OBJ(new_child, &ret, intern);
1249+
return;
1250+
cannot_add:
1251+
// TODO Convert to Error?
1252+
php_error_docref(NULL, E_WARNING, "Couldn't append node");
1253+
RETURN_FALSE;
12341254
}
12351255
/* }}} end dom_node_append_child */
12361256

‎ext/dom/parentnode.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,13 +298,14 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc)
298298
parentNode->children = newchild;
299299
}
300300

301-
parentNode->last = fragment->last;
301+
xmlNodePtr last = fragment->last;
302+
parentNode->last = last;
302303

303304
newchild->prev = prevsib;
304305

305306
dom_fragment_assign_parent_node(parentNode, fragment);
306307

307-
dom_reconcile_ns(parentNode->doc, newchild);
308+
dom_reconcile_ns_list(parentNode->doc, newchild, last);
308309
}
309310

310311
xmlFree(fragment);
@@ -335,13 +336,14 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc)
335336
nextsib = parentNode->children;
336337

337338
if (newchild) {
339+
xmlNodePtr last = fragment->last;
338340
parentNode->children = newchild;
339341
fragment->last->next = nextsib;
340-
nextsib->prev = fragment->last;
342+
nextsib->prev = last;
341343

342344
dom_fragment_assign_parent_node(parentNode, fragment);
343345

344-
dom_reconcile_ns(parentNode->doc, newchild);
346+
dom_reconcile_ns_list(parentNode->doc, newchild, last);
345347
}
346348

347349
xmlFree(fragment);
@@ -414,11 +416,13 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
414416
newchild = fragment->children;
415417

416418
if (newchild) {
419+
xmlNodePtr last = fragment->last;
420+
417421
/* Step 5: place fragment into the parent before viable_next_sibling */
418422
dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment);
419423

420424
dom_fragment_assign_parent_node(parentNode, fragment);
421-
dom_reconcile_ns(doc, newchild);
425+
dom_reconcile_ns_list(doc, newchild, last);
422426
}
423427

424428
xmlFree(fragment);
@@ -463,6 +467,8 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
463467
newchild = fragment->children;
464468

465469
if (newchild) {
470+
xmlNodePtr last = fragment->last;
471+
466472
/* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */
467473
if (!viable_previous_sibling) {
468474
viable_previous_sibling = parentNode->children;
@@ -473,7 +479,7 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
473479
dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment);
474480

475481
dom_fragment_assign_parent_node(parentNode, fragment);
476-
dom_reconcile_ns(doc, newchild);
482+
dom_reconcile_ns_list(doc, newchild, last);
477483
}
478484

479485
xmlFree(fragment);

‎ext/dom/php_dom.c

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,38 +1385,73 @@ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) {
13851385
}
13861386
/* }}} end dom_set_old_ns */
13871387

1388-
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
1388+
static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep)
13891389
{
13901390
xmlNsPtr nsptr, nsdftptr, curns, prevns = NULL;
13911391

1392-
if (nodep->type == XML_ELEMENT_NODE) {
1393-
/* Following if block primarily used for inserting nodes created via createElementNS */
1394-
if (nodep->nsDef != NULL) {
1395-
curns = nodep->nsDef;
1396-
while (curns) {
1397-
nsdftptr = curns->next;
1398-
if (curns->href != NULL) {
1399-
if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) &&
1400-
(curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) {
1401-
curns->next = NULL;
1402-
if (prevns == NULL) {
1403-
nodep->nsDef = nsdftptr;
1404-
} else {
1405-
prevns->next = nsdftptr;
1406-
}
1407-
dom_set_old_ns(doc, curns);
1408-
curns = prevns;
1392+
/* Following if block primarily used for inserting nodes created via createElementNS */
1393+
if (nodep->nsDef != NULL) {
1394+
curns = nodep->nsDef;
1395+
while (curns) {
1396+
nsdftptr = curns->next;
1397+
if (curns->href != NULL) {
1398+
if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) &&
1399+
(curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) {
1400+
curns->next = NULL;
1401+
if (prevns == NULL) {
1402+
nodep->nsDef = nsdftptr;
1403+
} else {
1404+
prevns->next = nsdftptr;
14091405
}
1406+
dom_set_old_ns(doc, curns);
1407+
curns = prevns;
14101408
}
1411-
prevns = curns;
1412-
curns = nsdftptr;
14131409
}
1410+
prevns = curns;
1411+
curns = nsdftptr;
14141412
}
1413+
}
1414+
}
1415+
1416+
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
1417+
{
1418+
if (nodep->type == XML_ELEMENT_NODE) {
1419+
dom_reconcile_ns_internal(doc, nodep);
14151420
xmlReconciliateNs(doc, nodep);
14161421
}
14171422
}
14181423
/* }}} */
14191424

1425+
static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
1426+
{
1427+
ZEND_ASSERT(nodep != NULL);
1428+
while (true) {
1429+
if (nodep->type == XML_ELEMENT_NODE) {
1430+
dom_reconcile_ns_internal(doc, nodep);
1431+
if (nodep->children) {
1432+
dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */);
1433+
}
1434+
}
1435+
if (nodep == last) {
1436+
break;
1437+
}
1438+
nodep = nodep->next;
1439+
}
1440+
}
1441+
1442+
void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
1443+
{
1444+
dom_reconcile_ns_list_internal(doc, nodep, last);
1445+
/* Outside of the recursion above because xmlReconciliateNs() performs its own recursion. */
1446+
while (true) {
1447+
xmlReconciliateNs(doc, nodep);
1448+
if (nodep == last) {
1449+
break;
1450+
}
1451+
nodep = nodep->next;
1452+
}
1453+
}
1454+
14201455
/*
14211456
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS
14221457

‎ext/dom/php_dom.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, i
110110
xmlNsPtr dom_get_ns(xmlNodePtr node, char *uri, int *errorcode, char *prefix);
111111
void dom_set_old_ns(xmlDoc *doc, xmlNs *ns);
112112
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep);
113+
void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last);
113114
xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName);
114115
void dom_normalize (xmlNodePtr nodep);
115116
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr nodep, char *ns, char *local, int *cur, int index);

0 commit comments

Comments
 (0)