Skip to content

Commit a73754f

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.4'
* PHP-8.4: Fix various document ref pointer mismanagements
2 parents ee41549 + 81a2cd4 commit a73754f

File tree

6 files changed

+111
-20
lines changed

6 files changed

+111
-20
lines changed

‎ext/dom/element.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -734,9 +734,8 @@ static void dom_element_set_attribute_node_common(INTERNAL_FUNCTION_PARAMETERS,
734734
xmlUnlinkNode((xmlNodePtr) attrp);
735735
}
736736

737-
if (attrp->doc == NULL && nodep->doc != NULL) {
738-
attrobj->document = intern->document;
739-
php_libxml_increment_doc_ref((php_libxml_node_object *)attrobj, NULL);
737+
if (attrp->doc == NULL && nodep->doc != NULL && intern->document != NULL) {
738+
dom_set_document_ref_pointers_attr(attrp, intern->document);
740739
}
741740

742741
xmlAddChild(nodep, (xmlNodePtr) attrp);

‎ext/dom/node.c

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -740,36 +740,61 @@ zend_result dom_node_text_content_write(dom_object *obj, zval *newval)
740740

741741
/* }}} */
742742

743-
/* Returns true if the node was changed, false otherwise. */
744-
static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
743+
/* Returns true if the node had the same document reference, false otherwise. */
744+
static bool dom_set_document_ref_obj_single(xmlNodePtr node, php_libxml_ref_obj *document)
745745
{
746746
dom_object *childobj = php_dom_object_get_data(node);
747-
if (childobj && !childobj->document) {
747+
if (!childobj) {
748+
return true;
749+
}
750+
if (!childobj->document) {
748751
childobj->document = document;
749752
document->refcount++;
750753
return true;
751754
}
752755
return false;
753756
}
754757

755-
static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
758+
void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document)
756759
{
757-
/* Applies the document to the entire subtree. */
758-
xmlSetTreeDoc(node, doc);
760+
ZEND_ASSERT(document != NULL);
759761

760-
if (!dom_set_document_ref_obj_single(node, doc, document)) {
761-
return;
762+
dom_set_document_ref_obj_single((xmlNodePtr) attr, document);
763+
for (xmlNodePtr attr_child = attr->children; attr_child; attr_child = attr_child->next) {
764+
dom_set_document_ref_obj_single(attr_child, document);
762765
}
766+
}
763767

764-
xmlNodePtr base = node;
765-
node = node->children;
766-
while (node != NULL) {
767-
ZEND_ASSERT(node != base);
768+
static bool dom_set_document_ref_pointers_node(xmlNodePtr node, php_libxml_ref_obj *document)
769+
{
770+
ZEND_ASSERT(document != NULL);
768771

769-
if (!dom_set_document_ref_obj_single(node, doc, document)) {
770-
break;
772+
if (!dom_set_document_ref_obj_single(node, document)) {
773+
return false;
774+
}
775+
776+
if (node->type == XML_ELEMENT_NODE) {
777+
for (xmlAttrPtr attr = node->properties; attr; attr = attr->next) {
778+
dom_set_document_ref_pointers_attr(attr, document);
771779
}
780+
}
781+
782+
return true;
783+
}
772784

785+
void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document)
786+
{
787+
if (!document) {
788+
return;
789+
}
790+
791+
if (!dom_set_document_ref_pointers_node(node, document)) {
792+
return;
793+
}
794+
795+
xmlNodePtr base = node;
796+
node = node->children;
797+
while (node != NULL && dom_set_document_ref_pointers_node(node, document)) {
773798
node = php_dom_next_in_tree_order(node, base);
774799
}
775800
}
@@ -860,7 +885,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
860885
}
861886

862887
if (child->doc == NULL && parentp->doc != NULL) {
863-
dom_set_document_pointers(child, parentp->doc, intern->document);
888+
dom_set_document_ref_pointers(child, intern->document);
864889
}
865890

866891
php_libxml_invalidate_node_list_cache(intern->document);
@@ -1167,7 +1192,7 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern)
11671192
}
11681193

11691194
if (newchild->doc == NULL && nodep->doc != NULL) {
1170-
dom_set_document_pointers(newchild, nodep->doc, intern->document);
1195+
dom_set_document_ref_pointers(newchild, intern->document);
11711196
}
11721197

11731198
if (newchild->type == XML_DOCUMENT_FRAG_NODE) {
@@ -1275,7 +1300,7 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern,
12751300
}
12761301

12771302
if (child->doc == NULL && nodep->doc != NULL) {
1278-
dom_set_document_pointers(child, nodep->doc, intern->document);
1303+
dom_set_document_ref_pointers(child, intern->document);
12791304
}
12801305

12811306
if (child->parent != NULL){

‎ext/dom/php_dom.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ bool dom_compare_value(const xmlAttr *attr, const xmlChar *value);
181181
void dom_attr_value_will_change(dom_object *obj, xmlAttrPtr attrp);
182182
bool php_dom_create_nullable_object(xmlNodePtr obj, zval *return_value, dom_object *domobj);
183183
xmlNodePtr dom_clone_node(php_dom_libxml_ns_mapper *ns_mapper, xmlNodePtr node, xmlDocPtr doc, bool recursive);
184+
void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document);
185+
void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document);
184186

185187
typedef enum {
186188
DOM_LOAD_STRING = 0,

‎ext/dom/tests/gh16336_1.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
GH-16336 (Attribute intern document mismanagement)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$doc = new DOMDocument();
9+
$elem = new DOMElement("g");
10+
$attr = new DOMAttr("iF", "j");
11+
12+
// First append, then attribute
13+
$doc->appendChild($elem);
14+
$elem->setAttributeNode($attr);
15+
echo $attr->firstChild->textContent;
16+
17+
?>
18+
--EXPECT--
19+
j

‎ext/dom/tests/gh16336_2.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
GH-16336 (Attribute intern document mismanagement)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$doc = new DOMDocument();
9+
$elem = new DOMElement("g");
10+
$attr = new DOMAttr("iF", "j");
11+
12+
// First attribute, then append
13+
$elem->setAttributeNode($attr);
14+
$doc->appendChild($elem);
15+
echo $attr->firstChild->textContent;
16+
17+
?>
18+
--EXPECT--
19+
j

‎ext/dom/tests/gh16338.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-16338 (Null-dereference in ext/dom/node.c)
3+
--EXTENSIONS--
4+
dom
5+
--CREDITS--
6+
chibinz
7+
--FILE--
8+
<?php
9+
$ref = new DOMEntityReference("G");
10+
$com = new DOMComment();
11+
$doc = new DOMDocument();
12+
$elem = new DOMElement("Rj", "o");
13+
$com2 = new DOMComment();
14+
$elem2 = new DOMElement("kx", null, "r");
15+
16+
$elem2->prepend($com);
17+
$com->before("Z");
18+
$com->before($com2);
19+
$com2->after($elem);
20+
$doc->insertBefore($elem2);
21+
$elem->insertBefore($ref);
22+
23+
echo $doc->saveXML();
24+
?>
25+
--EXPECT--
26+
<?xml version="1.0"?>
27+
<kx xmlns="r">Z<Rj>o&G;</Rj></kx>

0 commit comments

Comments
 (0)