Skip to content

Commit 74910b1

Browse files
tstarlingnielsdos
authored andcommitted
Factor out dom_remove_all_children()
A few callers remove all children of a node. The way it was done in node.c was unsafe, because it left nodep->last dangling. It just happens to not crash if xmlNodeSetContent() is called immediately afterwards.
1 parent 50fdad8 commit 74910b1

File tree

4 files changed

+16
-24
lines changed

4 files changed

+16
-24
lines changed

ext/dom/attr.c

+2-13
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ int dom_attr_value_write(dom_object *obj, zval *newval)
136136
{
137137
zend_string *str;
138138
xmlAttrPtr attrp = (xmlAttrPtr) dom_object_get_node(obj);
139-
xmlNodePtr node, next;
140139

141140
if (attrp == NULL) {
142141
php_dom_throw_error(INVALID_STATE_ERR, 1);
@@ -148,18 +147,8 @@ int dom_attr_value_write(dom_object *obj, zval *newval)
148147
return FAILURE;
149148
}
150149

151-
if (attrp->children) {
152-
node_list_unlink(attrp->children);
153-
node = attrp->children;
154-
while (node) {
155-
next = node->next;
156-
xmlUnlinkNode(node);
157-
xmlFreeNode(node);
158-
node = next;
159-
}
160-
}
161-
162-
node = xmlNewTextLen((xmlChar *) ZSTR_VAL(str), ZSTR_LEN(str));
150+
dom_remove_all_children((xmlNodePtr) attrp);
151+
xmlNodePtr node = xmlNewTextLen((xmlChar *) ZSTR_VAL(str), ZSTR_LEN(str));
163152
xmlAddChild((xmlNodePtr) attrp, node);
164153

165154
zend_string_release_ex(str, 0);

ext/dom/node.c

+2-11
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,7 @@ int dom_node_node_value_write(dom_object *obj, zval *newval)
179179
switch (nodep->type) {
180180
case XML_ELEMENT_NODE:
181181
case XML_ATTRIBUTE_NODE:
182-
if (nodep->children) {
183-
node_list_unlink(nodep->children);
184-
php_libxml_node_free_list((xmlNodePtr) nodep->children);
185-
nodep->children = NULL;
186-
}
182+
dom_remove_all_children(nodep);
187183
ZEND_FALLTHROUGH;
188184
case XML_TEXT_NODE:
189185
case XML_COMMENT_NODE:
@@ -783,12 +779,7 @@ int dom_node_text_content_write(dom_object *obj, zval *newval)
783779
* For the other cases, we *can* rely on xmlNodeSetContent because it is either a no-op, or handles
784780
* the content without encoding. */
785781
if (type == XML_DOCUMENT_FRAG_NODE || type == XML_ELEMENT_NODE || type == XML_ATTRIBUTE_NODE) {
786-
if (nodep->children) {
787-
node_list_unlink(nodep->children);
788-
php_libxml_node_free_list((xmlNodePtr) nodep->children);
789-
nodep->children = NULL;
790-
}
791-
782+
dom_remove_all_children(nodep);
792783
xmlNode *textNode = xmlNewText(xmlChars);
793784
xmlAddChild(nodep, textNode);
794785
} else {

ext/dom/php_dom.c

+10
Original file line numberDiff line numberDiff line change
@@ -1588,4 +1588,14 @@ static int dom_nodelist_has_dimension(zend_object *object, zval *member, int che
15881588
}
15891589
} /* }}} end dom_nodelist_has_dimension */
15901590

1591+
void dom_remove_all_children(xmlNodePtr nodep)
1592+
{
1593+
if (nodep->children) {
1594+
node_list_unlink(nodep->children);
1595+
php_libxml_node_free_list((xmlNodePtr) nodep->children);
1596+
nodep->children = NULL;
1597+
nodep->last = NULL;
1598+
}
1599+
}
1600+
15911601
#endif /* HAVE_DOM */

ext/dom/php_dom.h

+2
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ void dom_parent_node_before(dom_object *context, zval *nodes, uint32_t nodesc);
142142
void dom_child_node_remove(dom_object *context);
143143
void dom_child_replace_with(dom_object *context, zval *nodes, uint32_t nodesc);
144144

145+
void dom_remove_all_children(xmlNodePtr nodep);
146+
145147
#define DOM_GET_OBJ(__ptr, __id, __prtype, __intern) { \
146148
__intern = Z_DOMOBJ_P(__id); \
147149
if (__intern->ptr == NULL || !(__ptr = (__prtype)((php_libxml_node_ptr *)__intern->ptr)->node)) { \

0 commit comments

Comments
 (0)