Skip to content

Commit 0063ee4

Browse files
committed
Fix incorrect error handling in dom_zvals_to_fragment()
Discovered this pre-existing problem while testing phpGH-10682. Note: this problem existed *before* that PR. * Not all paths throw a hierarchy request error * xmlFreeNode must be used instead of xmlFree for the fragment to also free its children. * Free up nodes that couldn't be added when xmlAddChild fails. I unified the error handling code that's exactly the same with a goto to prevent at least some of such problems in the future.
1 parent 2d6decc commit 0063ee4

File tree

1 file changed

+12
-13
lines changed

1 file changed

+12
-13
lines changed

ext/dom/parentnode.c

+12-13
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
162162
newNode = dom_object_get_node(newNodeObj);
163163

164164
if (newNode->doc != documentNode) {
165-
xmlFree(fragment);
165+
xmlFreeNode(fragment);
166166
php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
167167
return NULL;
168168
}
@@ -175,10 +175,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
175175
xmlSetTreeDoc(newNode, documentNode);
176176

177177
if (newNode->type == XML_ATTRIBUTE_NODE) {
178-
xmlFree(fragment);
179-
180-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
181-
return NULL;
178+
goto hierarchy_request_err;
182179
}
183180

184181
/*
@@ -191,18 +188,16 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
191188
}
192189

193190
if (!xmlAddChild(fragment, newNode)) {
194-
xmlFree(fragment);
195191
if (nodesc > 1) {
196192
xmlFreeNode(newNode);
197193
}
198194

199-
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
200-
return NULL;
195+
goto hierarchy_request_err;
201196
}
202197

203198
continue;
204199
} else {
205-
xmlFree(fragment);
200+
xmlFreeNode(fragment);
206201

207202
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
208203
return NULL;
@@ -213,12 +208,11 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
213208
xmlSetTreeDoc(newNode, documentNode);
214209

215210
if (!xmlAddChild(fragment, newNode)) {
216-
xmlFree(fragment);
217-
218-
return NULL;
211+
xmlFreeNode(newNode);
212+
goto hierarchy_request_err;
219213
}
220214
} else {
221-
xmlFree(fragment);
215+
xmlFreeNode(fragment);
222216

223217
zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
224218

@@ -227,6 +221,11 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
227221
}
228222

229223
return fragment;
224+
225+
hierarchy_request_err:
226+
xmlFreeNode(fragment);
227+
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
228+
return NULL;
230229
}
231230

232231
static void dom_fragment_assign_parent_node(xmlNodePtr parentNode, xmlNodePtr fragment)

0 commit comments

Comments
 (0)