Skip to content

Commit b80ded8

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

25 files changed

+642
-124
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ PHP NEWS
1717
. Fixed bug GH-11791 (Wrong default value of DOMDocument::xmlStandalone).
1818
(nielsdos)
1919
. Fix json_encode result on DOMDocument. (nielsdos)
20+
. Fix manually calling __construct() on DOM classes. (nielsdos)
21+
. Fixed bug GH-11830 (ParentNode methods should perform their checks
22+
upfront). (nielsdos)
2023

2124
- FFI:
2225
. Fix leaking definitions when using FFI::cdef()->new(...). (ilutov)

‎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
@@ -1118,22 +1118,20 @@ PHP_METHOD(DOMDocument, __construct)
11181118
}
11191119

11201120
intern = Z_DOMOBJ_P(ZEND_THIS);
1121-
if (intern != NULL) {
1122-
olddoc = (xmlDocPtr) dom_object_get_node(intern);
1123-
if (olddoc != NULL) {
1124-
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
1125-
refcount = php_libxml_decrement_doc_ref((php_libxml_node_object *)intern);
1126-
if (refcount != 0) {
1127-
olddoc->_private = NULL;
1128-
}
1129-
}
1130-
intern->document = NULL;
1131-
if (php_libxml_increment_doc_ref((php_libxml_node_object *)intern, docp) == -1) {
1132-
/* docp is always non-null so php_libxml_increment_doc_ref() never returns -1 */
1133-
ZEND_UNREACHABLE();
1121+
olddoc = (xmlDocPtr) dom_object_get_node(intern);
1122+
if (olddoc != NULL) {
1123+
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
1124+
refcount = php_libxml_decrement_doc_ref((php_libxml_node_object *)intern);
1125+
if (refcount != 0) {
1126+
olddoc->_private = NULL;
11341127
}
1135-
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)docp, (void *)intern);
11361128
}
1129+
intern->document = NULL;
1130+
if (php_libxml_increment_doc_ref((php_libxml_node_object *)intern, docp) == -1) {
1131+
/* docp is always non-null so php_libxml_increment_doc_ref() never returns -1 */
1132+
ZEND_UNREACHABLE();
1133+
}
1134+
php_libxml_increment_node_ptr((php_libxml_node_object *)intern, (xmlNodePtr)docp, (void *)intern);
11371135
}
11381136
/* }}} end DOMDocument::__construct */
11391137

‎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: 79 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -141,107 +141,84 @@ static bool dom_is_node_in_list(const zval *nodes, int nodesc, const xmlNodePtr
141141
return false;
142142
}
143143

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+
144153
xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc)
145154
{
146155
int i;
147156
xmlDoc *documentNode;
148157
xmlNode *fragment;
149158
xmlNode *newNode;
150-
zend_class_entry *ce;
151159
dom_object *newNodeObj;
152-
int stricterror;
153-
154-
if (document == NULL) {
155-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1);
156-
return NULL;
157-
}
158160

159-
if (contextNode->type == XML_DOCUMENT_NODE || contextNode->type == XML_HTML_DOCUMENT_NODE) {
160-
documentNode = (xmlDoc *) contextNode;
161-
} else {
162-
documentNode = contextNode->doc;
163-
}
161+
documentNode = dom_doc_from_context_node(contextNode);
164162

165163
fragment = xmlNewDocFragment(documentNode);
166164

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

171-
stricterror = dom_get_strict_error(document);
172-
173169
for (i = 0; i < nodesc; i++) {
174170
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
175-
ce = Z_OBJCE(nodes[i]);
171+
newNodeObj = Z_DOMOBJ_P(&nodes[i]);
172+
newNode = dom_object_get_node(newNodeObj);
176173

177-
if (instanceof_function(ce, dom_node_class_entry)) {
178-
newNodeObj = Z_DOMOBJ_P(&nodes[i]);
179-
newNode = dom_object_get_node(newNodeObj);
180-
181-
if (newNode->doc != documentNode) {
182-
php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
183-
goto err;
184-
}
174+
if (newNode->parent != NULL) {
175+
xmlUnlinkNode(newNode);
176+
}
185177

186-
if (newNode->parent != NULL) {
187-
xmlUnlinkNode(newNode);
188-
}
178+
newNodeObj->document = document;
179+
xmlSetTreeDoc(newNode, documentNode);
189180

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

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

229211
xmlSetTreeDoc(newNode, documentNode);
230212

231213
if (!xmlAddChild(fragment, newNode)) {
232214
xmlFreeNode(newNode);
233-
goto hierarchy_request_err;
215+
goto err;
234216
}
235-
} else {
236-
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
237-
goto err;
238217
}
239218
}
240219

241220
return fragment;
242221

243-
hierarchy_request_err:
244-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
245222
err:
246223
xmlFreeNode(fragment);
247224
return NULL;
@@ -264,17 +241,39 @@ static void dom_fragment_assign_parent_node(xmlNodePtr parentNode, xmlNodePtr fr
264241
fragment->last = NULL;
265242
}
266243

267-
static zend_result dom_hierarchy_node_list(xmlNodePtr parentNode, zval *nodes, int nodesc)
244+
static zend_result dom_sanity_check_node_list_for_insertion(php_libxml_ref_obj *document, xmlNodePtr parentNode, zval *nodes, int nodesc)
268245
{
246+
if (document == NULL) {
247+
php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1);
248+
return FAILURE;
249+
}
250+
251+
xmlDocPtr documentNode = dom_doc_from_context_node(parentNode);
252+
269253
for (int i = 0; i < nodesc; i++) {
270-
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
254+
zend_uchar type = Z_TYPE(nodes[i]);
255+
if (type == IS_OBJECT) {
271256
const zend_class_entry *ce = Z_OBJCE(nodes[i]);
272257

273258
if (instanceof_function(ce, dom_node_class_entry)) {
274-
if (dom_hierarchy(parentNode, dom_object_get_node(Z_DOMOBJ_P(nodes + i))) != SUCCESS) {
259+
xmlNodePtr node = dom_object_get_node(Z_DOMOBJ_P(nodes + i));
260+
261+
if (node->doc != documentNode) {
262+
php_dom_throw_error(WRONG_DOCUMENT_ERR, dom_get_strict_error(document));
275263
return FAILURE;
276264
}
265+
266+
if (node->type == XML_ATTRIBUTE_NODE || dom_hierarchy(parentNode, node) != SUCCESS) {
267+
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(document));
268+
return FAILURE;
269+
}
270+
} else {
271+
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
272+
return FAILURE;
277273
}
274+
} else if (type != IS_STRING) {
275+
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
276+
return FAILURE;
278277
}
279278
}
280279

@@ -286,8 +285,7 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc)
286285
xmlNode *parentNode = dom_object_get_node(context);
287286
xmlNodePtr newchild, prevsib;
288287

289-
if (UNEXPECTED(dom_hierarchy_node_list(parentNode, nodes, nodesc) != SUCCESS)) {
290-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, dom_get_strict_error(context->document));
288+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
291289
return;
292290
}
293291

@@ -329,8 +327,7 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc)
329327
return;
330328
}
331329

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

@@ -414,6 +411,10 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
414411

415412
doc = prevsib->doc;
416413

414+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
415+
return;
416+
}
417+
417418
/* Spec step 4: convert nodes into fragment */
418419
fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
419420

@@ -465,6 +466,10 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
465466

466467
doc = nextsib->doc;
467468

469+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
470+
return;
471+
}
472+
468473
/* Spec step 4: convert nodes into fragment */
469474
fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
470475

@@ -555,6 +560,10 @@ void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc)
555560

556561
xmlNodePtr insertion_point = child->next;
557562

563+
if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) {
564+
return;
565+
}
566+
558567
xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
559568
if (UNEXPECTED(fragment == NULL)) {
560569
return;

‎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)