Skip to content

Commit 3ad5029

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.2'
* PHP-8.2: Fix GH-11830: ParentNode methods should perform their checks upfront Fix manually calling __construct() on DOM classes
2 parents e701b2f + b80ded8 commit 3ad5029

24 files changed

+641
-127
lines changed

‎ext/dom/attr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ PHP_METHOD(DOMAttr, __construct)
6262

6363
oldnode = dom_object_get_node(intern);
6464
if (oldnode != NULL) {
65-
php_libxml_node_free_resource(oldnode );
65+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
6666
}
6767
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)nodep, (void *)intern);
6868
}

‎ext/dom/cdatasection.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ PHP_METHOD(DOMCdataSection, __construct)
5252
intern = Z_DOMOBJ_P(ZEND_THIS);
5353
oldnode = dom_object_get_node(intern);
5454
if (oldnode != NULL) {
55-
php_libxml_node_free_resource(oldnode );
55+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
5656
}
5757
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
5858
}

‎ext/dom/comment.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,11 @@ PHP_METHOD(DOMComment, __construct)
5050
}
5151

5252
intern = Z_DOMOBJ_P(ZEND_THIS);
53-
if (intern != NULL) {
54-
oldnode = dom_object_get_node(intern);
55-
if (oldnode != NULL) {
56-
php_libxml_node_free_resource(oldnode );
57-
}
58-
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)nodep, (void *)intern);
53+
oldnode = dom_object_get_node(intern);
54+
if (oldnode != NULL) {
55+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
5956
}
57+
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)nodep, (void *)intern);
6058
}
6159
/* }}} end DOMComment::__construct */
6260

‎ext/dom/document.c

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,22 +1127,20 @@ PHP_METHOD(DOMDocument, __construct)
11271127
}
11281128

11291129
intern = Z_DOMOBJ_P(ZEND_THIS);
1130-
if (intern != NULL) {
1131-
olddoc = (xmlDocPtr) dom_object_get_node(intern);
1132-
if (olddoc != NULL) {
1133-
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
1134-
refcount = php_libxml_decrement_doc_ref((php_libxml_node_object *)intern);
1135-
if (refcount != 0) {
1136-
olddoc->_private = NULL;
1137-
}
1130+
olddoc = (xmlDocPtr) dom_object_get_node(intern);
1131+
if (olddoc != NULL) {
1132+
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
1133+
refcount = php_libxml_decrement_doc_ref((php_libxml_node_object *)intern);
1134+
if (refcount != 0) {
1135+
olddoc->_private = NULL;
11381136
}
1139-
intern->document = NULL;
1140-
if (php_libxml_increment_doc_ref((php_libxml_node_object *)intern, docp) == -1) {
1141-
/* docp is always non-null so php_libxml_increment_doc_ref() never returns -1 */
1142-
ZEND_UNREACHABLE();
1143-
}
1144-
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)docp, (void *)intern);
11451137
}
1138+
intern->document = NULL;
1139+
if (php_libxml_increment_doc_ref((php_libxml_node_object *)intern, docp) == -1) {
1140+
/* docp is always non-null so php_libxml_increment_doc_ref() never returns -1 */
1141+
ZEND_UNREACHABLE();
1142+
}
1143+
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)docp, (void *)intern);
11461144
}
11471145
/* }}} end DOMDocument::__construct */
11481146

‎ext/dom/documentfragment.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,8 @@ PHP_METHOD(DOMDocumentFragment, __construct)
5050
intern = Z_DOMOBJ_P(ZEND_THIS);
5151
oldnode = dom_object_get_node(intern);
5252
if (oldnode != NULL) {
53-
php_libxml_node_free_resource(oldnode );
53+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
5454
}
55-
/* php_dom_set_object(intern, nodep); */
5655
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
5756
}
5857
/* }}} end DOMDocumentFragment::__construct */

‎ext/dom/element.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ PHP_METHOD(DOMElement, __construct)
9797
intern = Z_DOMOBJ_P(ZEND_THIS);
9898
oldnode = dom_object_get_node(intern);
9999
if (oldnode != NULL) {
100-
php_libxml_node_free_resource(oldnode );
100+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
101101
}
102102
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
103103
}

‎ext/dom/entityreference.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,11 @@ PHP_METHOD(DOMEntityReference, __construct)
5757
}
5858

5959
intern = Z_DOMOBJ_P(ZEND_THIS);
60-
if (intern != NULL) {
61-
oldnode = dom_object_get_node(intern);
62-
if (oldnode != NULL) {
63-
php_libxml_node_free_resource(oldnode );
64-
}
65-
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, node, (void *)intern);
60+
oldnode = dom_object_get_node(intern);
61+
if (oldnode != NULL) {
62+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
6663
}
64+
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, node, (void *)intern);
6765
}
6866
/* }}} end DOMEntityReference::__construct */
6967

‎ext/dom/parentnode.c

Lines changed: 81 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -141,104 +141,81 @@ static bool dom_is_node_in_list(const zval *nodes, uint32_t nodesc, const xmlNod
141141
return false;
142142
}
143143

144-
xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, uint32_t nodesc)
144+
static xmlDocPtr dom_doc_from_context_node(xmlNodePtr contextNode)
145+
{
146+
if (contextNode->type == XML_DOCUMENT_NODE || contextNode->type == XML_HTML_DOCUMENT_NODE) {
147+
return (xmlDocPtr) contextNode;
148+
} else {
149+
return contextNode->doc;
150+
}
151+
}
152+
153+
xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc)
145154
{
146155
xmlDoc *documentNode;
147156
xmlNode *fragment;
148157
xmlNode *newNode;
149-
zend_class_entry *ce;
150158
dom_object *newNodeObj;
151-
int stricterror;
152159

153-
if (document == NULL) {
154-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1);
155-
return NULL;
156-
}
157-
158-
if (contextNode->type == XML_DOCUMENT_NODE || contextNode->type == XML_HTML_DOCUMENT_NODE) {
159-
documentNode = (xmlDoc *) contextNode;
160-
} else {
161-
documentNode = contextNode->doc;
162-
}
160+
documentNode = dom_doc_from_context_node(contextNode);
163161

164162
fragment = xmlNewDocFragment(documentNode);
165163

166164
if (!fragment) {
167165
return NULL;
168166
}
169167

170-
stricterror = dom_get_strict_error(document);
171-
172168
for (uint32_t i = 0; i < nodesc; i++) {
173169
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
174-
ce = Z_OBJCE(nodes[i]);
175-
176-
if (instanceof_function(ce, dom_node_class_entry)) {
177-
newNodeObj = Z_DOMOBJ_P(&nodes[i]);
178-
newNode = dom_object_get_node(newNodeObj);
170+
newNodeObj = Z_DOMOBJ_P(&nodes[i]);
171+
newNode = dom_object_get_node(newNodeObj);
179172

180-
if (newNode->doc != documentNode) {
181-
php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
182-
goto err;
183-
}
173+
if (newNode->parent != NULL) {
174+
xmlUnlinkNode(newNode);
175+
}
184176

185-
if (newNode->parent != NULL) {
186-
xmlUnlinkNode(newNode);
187-
}
177+
newNodeObj->document = document;
178+
xmlSetTreeDoc(newNode, documentNode);
188179

189-
newNodeObj->document = document;
190-
xmlSetTreeDoc(newNode, documentNode);
180+
/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
181+
* "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)".
182+
* So we must take a copy if this situation arises to prevent a use-after-free. */
183+
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
184+
if (will_free) {
185+
newNode = xmlCopyNode(newNode, 1);
186+
}
191187

192-
if (newNode->type == XML_ATTRIBUTE_NODE) {
193-
goto hierarchy_request_err;
188+
if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
189+
/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
190+
newNode = newNode->children;
191+
while (newNode) {
192+
xmlNodePtr next = newNode->next;
193+
xmlUnlinkNode(newNode);
194+
if (!xmlAddChild(fragment, newNode)) {
195+
goto err;
196+
}
197+
newNode = next;
194198
}
195-
196-
/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
197-
* "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)".
198-
* So we must take a copy if this situation arises to prevent a use-after-free. */
199-
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
199+
} else if (!xmlAddChild(fragment, newNode)) {
200200
if (will_free) {
201-
newNode = xmlCopyNode(newNode, 1);
202-
}
203-
204-
if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
205-
/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
206-
newNode = newNode->children;
207-
while (newNode) {
208-
xmlNodePtr next = newNode->next;
209-
xmlUnlinkNode(newNode);
210-
if (!xmlAddChild(fragment, newNode)) {
211-
goto hierarchy_request_err;
212-
}
213-
newNode = next;
214-
}
215-
} else if (!xmlAddChild(fragment, newNode)) {
216-
if (will_free) {
217-
xmlFreeNode(newNode);
218-
}
219-
goto hierarchy_request_err;
201+
xmlFreeNode(newNode);
220202
}
221-
} else {
222-
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_value_name(&nodes[i]));
223203
goto err;
224204
}
225-
} else if (Z_TYPE(nodes[i]) == IS_STRING) {
205+
} else {
206+
ZEND_ASSERT(Z_TYPE(nodes[i]) == IS_STRING);
207+
226208
newNode = xmlNewDocText(documentNode, (xmlChar *) Z_STRVAL(nodes[i]));
227209

228210
if (!xmlAddChild(fragment, newNode)) {
229211
xmlFreeNode(newNode);
230-
goto hierarchy_request_err;
212+
goto err;
231213
}
232-
} else {
233-
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_value_name(&nodes[i]));
234-
goto err;
235214
}
236215
}
237216

238217
return fragment;
239218

240-
hierarchy_request_err:
241-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
242219
err:
243220
xmlFreeNode(fragment);
244221
return NULL;
@@ -261,17 +238,39 @@ static void dom_fragment_assign_parent_node(xmlNodePtr parentNode, xmlNodePtr fr
261238
fragment->last = NULL;
262239
}
263240

264-
static zend_result dom_hierarchy_node_list(xmlNodePtr parentNode, zval *nodes, uint32_t nodesc)
241+
static zend_result dom_sanity_check_node_list_for_insertion(php_libxml_ref_obj *document, xmlNodePtr parentNode, zval *nodes, int nodesc)
265242
{
243+
if (document == NULL) {
244+
php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1);
245+
return FAILURE;
246+
}
247+
248+
xmlDocPtr documentNode = dom_doc_from_context_node(parentNode);
249+
266250
for (uint32_t i = 0; i < nodesc; i++) {
267-
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
251+
zend_uchar type = Z_TYPE(nodes[i]);
252+
if (type == IS_OBJECT) {
268253
const zend_class_entry *ce = Z_OBJCE(nodes[i]);
269254

270255
if (instanceof_function(ce, dom_node_class_entry)) {
271-
if (dom_hierarchy(parentNode, dom_object_get_node(Z_DOMOBJ_P(nodes + i))) != SUCCESS) {
256+
xmlNodePtr node = dom_object_get_node(Z_DOMOBJ_P(nodes + i));
257+
258+
if (node->doc != documentNode) {
259+
php_dom_throw_error(WRONG_DOCUMENT_ERR, dom_get_strict_error(document));
260+
return FAILURE;
261+
}
262+
263+
if (node->type == XML_ATTRIBUTE_NODE || dom_hierarchy(parentNode, node) != SUCCESS) {
264+
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(document));
272265
return FAILURE;
273266
}
267+
} else {
268+
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
269+
return FAILURE;
274270
}
271+
} else if (type != IS_STRING) {
272+
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
273+
return FAILURE;
275274
}
276275
}
277276

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

286-
if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) {
287-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document));
285+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
288286
return;
289287
}
290288

@@ -328,8 +326,7 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc)
328326
return;
329327
}
330328

331-
if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) {
332-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document));
329+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
333330
return;
334331
}
335332

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

416413
doc = prevsib->doc;
417414

415+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
416+
return;
417+
}
418+
418419
php_libxml_invalidate_node_list_cache_from_doc(doc);
419420

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

469470
doc = nextsib->doc;
470471

472+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
473+
return;
474+
}
475+
471476
php_libxml_invalidate_node_list_cache_from_doc(doc);
472477

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

555560
xmlNodePtr insertion_point = child->next;
556561

562+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
563+
return;
564+
}
565+
557566
xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
558567
if (UNEXPECTED(fragment == NULL)) {
559568
return;
@@ -586,8 +595,7 @@ void dom_parent_node_replace_children(dom_object *context, zval *nodes, uint32_t
586595

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

‎ext/dom/processinginstruction.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ PHP_METHOD(DOMProcessingInstruction, __construct)
5959
intern = Z_DOMOBJ_P(ZEND_THIS);
6060
oldnode = dom_object_get_node(intern);
6161
if (oldnode != NULL) {
62-
php_libxml_node_free_resource(oldnode );
62+
php_libxml_node_decrement_resource((php_libxml_node_object *)intern);
6363
}
6464
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, nodep, (void *)intern);
6565
}

‎ext/dom/tests/DOMDocumentFragment_construct_basic_002.phpt

Lines changed: 0 additions & 16 deletions
This file was deleted.

0 commit comments

Comments
 (0)