diff --git a/ext/dom/element.c b/ext/dom/element.c index 0b4117fb08ea0..78487b0dfd911 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -642,9 +642,8 @@ PHP_METHOD(DOMElement, setAttributeNode) xmlUnlinkNode((xmlNodePtr) attrp); } - if (attrp->doc == NULL && nodep->doc != NULL) { - attrobj->document = intern->document; - php_libxml_increment_doc_ref((php_libxml_node_object *)attrobj, NULL); + if (attrp->doc == NULL && nodep->doc != NULL && intern->document != NULL) { + dom_set_document_ref_pointers_attr(attrp, intern->document); } xmlAddChild(nodep, (xmlNodePtr) attrp); diff --git a/ext/dom/node.c b/ext/dom/node.c index 48e3c5cc2a747..ec1210570b08f 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -820,11 +820,14 @@ int dom_node_text_content_write(dom_object *obj, zval *newval) /* }}} */ -/* Returns true if the node was changed, false otherwise. */ -static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document) +/* Returns true if the node had the same document reference, false otherwise. */ +static bool dom_set_document_ref_obj_single(xmlNodePtr node, php_libxml_ref_obj *document) { dom_object *childobj = php_dom_object_get_data(node); - if (childobj && !childobj->document) { + if (!childobj) { + return true; + } + if (!childobj->document) { childobj->document = document; document->refcount++; return true; @@ -832,13 +835,41 @@ static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_ return false; } +void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document) +{ + ZEND_ASSERT(document != NULL); + + dom_set_document_ref_obj_single((xmlNodePtr) attr, document); + for (xmlNodePtr attr_child = attr->children; attr_child; attr_child = attr_child->next) { + dom_set_document_ref_obj_single(attr_child, document); + } +} + +static bool dom_set_document_ref_pointers_node(xmlNodePtr node, php_libxml_ref_obj *document) +{ + ZEND_ASSERT(document != NULL); + + if (!dom_set_document_ref_obj_single(node, document)) { + return false; + } + + if (node->type == XML_ELEMENT_NODE) { + for (xmlAttrPtr attr = node->properties; attr; attr = attr->next) { + dom_set_document_ref_pointers_attr(attr, document); + } + } + + return true; +} + /* TODO: on 8.4 replace the loop with the tree walk helper function. */ -static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document) +void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document) { - /* Applies the document to the entire subtree. */ - xmlSetTreeDoc(node, doc); + if (!document) { + return; + } - if (!dom_set_document_ref_obj_single(node, doc, document)) { + if (!dom_set_document_ref_pointers_node(node, document)) { return; } @@ -847,7 +878,7 @@ static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml while (node != NULL) { ZEND_ASSERT(node != base); - if (!dom_set_document_ref_obj_single(node, doc, document)) { + if (!dom_set_document_ref_pointers_node(node, document)) { break; } @@ -974,7 +1005,7 @@ PHP_METHOD(DOMNode, insertBefore) } if (child->doc == NULL && parentp->doc != NULL) { - dom_set_document_pointers(child, parentp->doc, intern->document); + dom_set_document_ref_pointers(child, intern->document); } php_libxml_invalidate_node_list_cache(intern->document); @@ -1149,7 +1180,7 @@ PHP_METHOD(DOMNode, replaceChild) } if (newchild->doc == NULL && nodep->doc != NULL) { - dom_set_document_pointers(newchild, nodep->doc, intern->document); + dom_set_document_ref_pointers(newchild, intern->document); } if (newchild->type == XML_DOCUMENT_FRAG_NODE) { @@ -1252,7 +1283,7 @@ PHP_METHOD(DOMNode, appendChild) } if (child->doc == NULL && nodep->doc != NULL) { - dom_set_document_pointers(child, nodep->doc, intern->document); + dom_set_document_ref_pointers(child, intern->document); } if (child->parent != NULL){ diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index c5c888a64c58b..fe0c5471c6ca4 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -154,6 +154,8 @@ bool php_dom_is_node_connected(const xmlNode *node); bool php_dom_adopt_node(xmlNodePtr nodep, dom_object *dom_object_new_document, xmlDocPtr new_document); xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri); xmlEntityPtr dom_entity_reference_fetch_and_sync_declaration(xmlNodePtr reference); +void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document); +void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document); /* parentnode */ void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc); diff --git a/ext/dom/tests/gh16336_1.phpt b/ext/dom/tests/gh16336_1.phpt new file mode 100644 index 0000000000000..c4a797a12ffc5 --- /dev/null +++ b/ext/dom/tests/gh16336_1.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-16336 (Attribute intern document mismanagement) +--EXTENSIONS-- +dom +--FILE-- +appendChild($elem); +$elem->setAttributeNode($attr); +echo $attr->firstChild->textContent; + +?> +--EXPECT-- +j diff --git a/ext/dom/tests/gh16336_2.phpt b/ext/dom/tests/gh16336_2.phpt new file mode 100644 index 0000000000000..f579c0e46e362 --- /dev/null +++ b/ext/dom/tests/gh16336_2.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-16336 (Attribute intern document mismanagement) +--EXTENSIONS-- +dom +--FILE-- +setAttributeNode($attr); +$doc->appendChild($elem); +echo $attr->firstChild->textContent; + +?> +--EXPECT-- +j diff --git a/ext/dom/tests/gh16338.phpt b/ext/dom/tests/gh16338.phpt new file mode 100644 index 0000000000000..7346f62107ee1 --- /dev/null +++ b/ext/dom/tests/gh16338.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-16338 (Null-dereference in ext/dom/node.c) +--EXTENSIONS-- +dom +--CREDITS-- +chibinz +--FILE-- +prepend($com); +$com->before("Z"); +$com->before($com2); +$com2->after($elem); +$doc->insertBefore($elem2); +$elem->insertBefore($ref); + +echo $doc->saveXML(); +?> +--EXPECT-- + +Zo&G;