Skip to content

Commit

Permalink
Merge branch 'PHP-8.2'
Browse files Browse the repository at this point in the history 
* PHP-8.2:
  Fix GH-11830: ParentNode methods should perform their checks upfront
  Fix manually calling __construct() on DOM classes
  • Loading branch information
@nielsdos
nielsdos committed Aug 7, 2023
2 parents e701b2f + b80ded8 commit 3ad5029
Show file tree
Hide file tree
Showing 24 changed files with 641 additions and 127 deletions.
2 changes: 1 addition & 1 deletion ext/dom/attr.c
View file
Expand Up @@ -62,7 +62,7 @@ PHP_METHOD(DOMAttr, __construct)

oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)nodep, (void *)intern);
}
Expand Down
2 changes: 1 addition & 1 deletion ext/dom/cdatasection.c
View file
Expand Up @@ -52,7 +52,7 @@ PHP_METHOD(DOMCdataSection, __construct)
intern = Z_DOMOBJ_P(ZEND_THIS);
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
}
Expand Down
10 changes: 4 additions & 6 deletions ext/dom/comment.c
View file
Expand Up @@ -50,13 +50,11 @@ PHP_METHOD(DOMComment, __construct)
}

intern = Z_DOMOBJ_P(ZEND_THIS);
if (intern != NULL) {
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)nodep, (void *)intern);
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)nodep, (void *)intern);
}
/* }}} end DOMComment::__construct */

Expand Down
26 changes: 12 additions & 14 deletions ext/dom/document.c
View file
Expand Up @@ -1127,22 +1127,20 @@ PHP_METHOD(DOMDocument, __construct)
}

intern = Z_DOMOBJ_P(ZEND_THIS);
if (intern != NULL) {
olddoc = (xmlDocPtr) dom_object_get_node(intern);
if (olddoc != NULL) {
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
refcount = php_libxml_decrement_doc_ref((php_libxml_node_object *)intern);
if (refcount != 0) {
olddoc->_private = NULL;
}
olddoc = (xmlDocPtr) dom_object_get_node(intern);
if (olddoc != NULL) {
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
refcount = php_libxml_decrement_doc_ref((php_libxml_node_object *)intern);
if (refcount != 0) {
olddoc->_private = NULL;
}
intern->document = NULL;
if (php_libxml_increment_doc_ref((php_libxml_node_object *)intern, docp) == -1) {
/* docp is always non-null so php_libxml_increment_doc_ref() never returns -1 */
ZEND_UNREACHABLE();
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)docp, (void *)intern);
}
intern->document = NULL;
if (php_libxml_increment_doc_ref((php_libxml_node_object *)intern, docp) == -1) {
/* docp is always non-null so php_libxml_increment_doc_ref() never returns -1 */
ZEND_UNREACHABLE();
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)docp, (void *)intern);
}
/* }}} end DOMDocument::__construct */

Expand Down
3 changes: 1 addition & 2 deletions ext/dom/documentfragment.c
View file
Expand Up @@ -50,9 +50,8 @@ PHP_METHOD(DOMDocumentFragment, __construct)
intern = Z_DOMOBJ_P(ZEND_THIS);
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
/* php_dom_set_object(intern, nodep); */
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
}
/* }}} end DOMDocumentFragment::__construct */
Expand Down
2 changes: 1 addition & 1 deletion ext/dom/element.c
View file
Expand Up @@ -97,7 +97,7 @@ PHP_METHOD(DOMElement, __construct)
intern = Z_DOMOBJ_P(ZEND_THIS);
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
}
Expand Down
10 changes: 4 additions & 6 deletions ext/dom/entityreference.c
View file
Expand Up @@ -57,13 +57,11 @@ PHP_METHOD(DOMEntityReference, __construct)
}

intern = Z_DOMOBJ_P(ZEND_THIS);
if (intern != NULL) {
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, node, (void *)intern);
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, node, (void *)intern);
}
/* }}} end DOMEntityReference::__construct */

Expand Down
154 changes: 81 additions & 73 deletions ext/dom/parentnode.c
View file
Expand Up @@ -141,104 +141,81 @@ static bool dom_is_node_in_list(const zval *nodes, uint32_t nodesc, const xmlNod
return false;
}

xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, uint32_t nodesc)
static xmlDocPtr dom_doc_from_context_node(xmlNodePtr contextNode)
{
if (contextNode->type == XML_DOCUMENT_NODE || contextNode->type == XML_HTML_DOCUMENT_NODE) {
return (xmlDocPtr) contextNode;
} else {
return contextNode->doc;
}
}

xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc)
{
xmlDoc *documentNode;
xmlNode *fragment;
xmlNode *newNode;
zend_class_entry *ce;
dom_object *newNodeObj;
int stricterror;

if (document == NULL) {
php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1);
return NULL;
}

if (contextNode->type == XML_DOCUMENT_NODE || contextNode->type == XML_HTML_DOCUMENT_NODE) {
documentNode = (xmlDoc *) contextNode;
} else {
documentNode = contextNode->doc;
}
documentNode = dom_doc_from_context_node(contextNode);

fragment = xmlNewDocFragment(documentNode);

if (!fragment) {
return NULL;
}

stricterror = dom_get_strict_error(document);

for (uint32_t i = 0; i < nodesc; i++) {
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
ce = Z_OBJCE(nodes[i]);

if (instanceof_function(ce, dom_node_class_entry)) {
newNodeObj = Z_DOMOBJ_P(&nodes[i]);
newNode = dom_object_get_node(newNodeObj);
newNodeObj = Z_DOMOBJ_P(&nodes[i]);
newNode = dom_object_get_node(newNodeObj);

if (newNode->doc != documentNode) {
php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
goto err;
}
if (newNode->parent != NULL) {
xmlUnlinkNode(newNode);
}

if (newNode->parent != NULL) {
xmlUnlinkNode(newNode);
}
newNodeObj->document = document;
xmlSetTreeDoc(newNode, documentNode);

newNodeObj->document = document;
xmlSetTreeDoc(newNode, documentNode);
/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
* "Add a new node to @parent, at the end of the child (or property) list merging adjacent TEXT nodes (in which case @cur is freed)".
* So we must take a copy if this situation arises to prevent a use-after-free. */
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
if (will_free) {
newNode = xmlCopyNode(newNode, 1);
}

if (newNode->type == XML_ATTRIBUTE_NODE) {
goto hierarchy_request_err;
if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
newNode = newNode->children;
while (newNode) {
xmlNodePtr next = newNode->next;
xmlUnlinkNode(newNode);
if (!xmlAddChild(fragment, newNode)) {
goto err;
}
newNode = next;
}

/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
* "Add a new node to @parent, at the end of the child (or property) list merging adjacent TEXT nodes (in which case @cur is freed)".
* So we must take a copy if this situation arises to prevent a use-after-free. */
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
} else if (!xmlAddChild(fragment, newNode)) {
if (will_free) {
newNode = xmlCopyNode(newNode, 1);
}

if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
newNode = newNode->children;
while (newNode) {
xmlNodePtr next = newNode->next;
xmlUnlinkNode(newNode);
if (!xmlAddChild(fragment, newNode)) {
goto hierarchy_request_err;
}
newNode = next;
}
} else if (!xmlAddChild(fragment, newNode)) {
if (will_free) {
xmlFreeNode(newNode);
}
goto hierarchy_request_err;
xmlFreeNode(newNode);
}
} else {
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_value_name(&nodes[i]));
goto err;
}
} else if (Z_TYPE(nodes[i]) == IS_STRING) {
} else {
ZEND_ASSERT(Z_TYPE(nodes[i]) == IS_STRING);

newNode = xmlNewDocText(documentNode, (xmlChar *) Z_STRVAL(nodes[i]));

if (!xmlAddChild(fragment, newNode)) {
xmlFreeNode(newNode);
goto hierarchy_request_err;
goto err;
}
} else {
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_value_name(&nodes[i]));
goto err;
}
}

return fragment;

hierarchy_request_err:
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
err:
xmlFreeNode(fragment);
return NULL;
Expand All @@ -261,17 +238,39 @@ static void dom_fragment_assign_parent_node(xmlNodePtr parentNode, xmlNodePtr fr
fragment->last = NULL;
}

static zend_result dom_hierarchy_node_list(xmlNodePtr parentNode, zval *nodes, uint32_t nodesc)
static zend_result dom_sanity_check_node_list_for_insertion(php_libxml_ref_obj *document, xmlNodePtr parentNode, zval *nodes, int nodesc)
{
if (document == NULL) {
php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1);
return FAILURE;
}

xmlDocPtr documentNode = dom_doc_from_context_node(parentNode);

for (uint32_t i = 0; i < nodesc; i++) {
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
zend_uchar type = Z_TYPE(nodes[i]);
if (type == IS_OBJECT) {
const zend_class_entry *ce = Z_OBJCE(nodes[i]);

if (instanceof_function(ce, dom_node_class_entry)) {
if (dom_hierarchy(parentNode, dom_object_get_node(Z_DOMOBJ_P(nodes + i))) != SUCCESS) {
xmlNodePtr node = dom_object_get_node(Z_DOMOBJ_P(nodes + i));

if (node->doc != documentNode) {
php_dom_throw_error(WRONG_DOCUMENT_ERR, dom_get_strict_error(document));
return FAILURE;
}

if (node->type == XML_ATTRIBUTE_NODE || dom_hierarchy(parentNode, node) != SUCCESS) {
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(document));
return FAILURE;
}
} else {
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
return FAILURE;
}
} else if (type != IS_STRING) {
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
return FAILURE;
}
}

Expand All @@ -283,8 +282,7 @@ void dom_parent_node_append(dom_object *context, zval *nodes, uint32_t nodesc)
xmlNode *parentNode = dom_object_get_node(context);
xmlNodePtr newchild, prevsib;

if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) {
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document));
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
return;
}

Expand Down Expand Up @@ -328,8 +326,7 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc)
return;
}

if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) {
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document));
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
return;
}

Expand Down Expand Up @@ -415,6 +412,10 @@ void dom_parent_node_after(dom_object *context, zval *nodes, uint32_t nodesc)

doc = prevsib->doc;

if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
return;
}

php_libxml_invalidate_node_list_cache_from_doc(doc);

/* Spec step 4: convert nodes into fragment */
Expand Down Expand Up @@ -468,6 +469,10 @@ void dom_parent_node_before(dom_object *context, zval *nodes, uint32_t nodesc)

doc = nextsib->doc;

if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
return;
}

php_libxml_invalidate_node_list_cache_from_doc(doc);

/* Spec step 4: convert nodes into fragment */
Expand Down Expand Up @@ -554,6 +559,10 @@ void dom_child_replace_with(dom_object *context, zval *nodes, uint32_t nodesc)

xmlNodePtr insertion_point = child->next;

if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
return;
}

xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
if (UNEXPECTED(fragment == NULL)) {
return;
Expand Down Expand Up @@ -586,8 +595,7 @@ void dom_parent_node_replace_children(dom_object *context, zval *nodes, uint32_t

xmlNodePtr thisp = dom_object_get_node(context);
/* Note: Only rule 2 of pre-insertion validity can be broken */
if (dom_hierarchy_node_list(thisp, nodes, nodesc)) {
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document));
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, thisp, nodes, nodesc) != SUCCESS)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion ext/dom/processinginstruction.c
View file
Expand Up @@ -59,7 +59,7 @@ PHP_METHOD(DOMProcessingInstruction, __construct)
intern = Z_DOMOBJ_P(ZEND_THIS);
oldnode = dom_object_get_node(intern);
if (oldnode != NULL) {
php_libxml_node_free_resource(oldnode );
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
}
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
}
Expand Down
16 changes: 0 additions & 16 deletions ext/dom/tests/DOMDocumentFragment_construct_basic_002.phpt
View file

This file was deleted.

0 comments on commit 3ad5029

Please sign in to comment.