Skip to content

Commit 9c36a95

Browse files
nielsdosnielsdos
committed
Improve destruction code
1 parent 35e0c57 commit 9c36a95

File tree

11 files changed

+116
-21
lines changed

11 files changed

+116
-21
lines changed

‎ext/dom/html5_parser.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,12 @@ static lexbor_libxml2_bridge_status lexbor_libxml2_bridge_convert(
171171
break;
172172
}
173173

174+
if (dom_add_element_destruct_callback_marker(lxml_element, private_data) != SUCCESS) {
175+
xmlFreeNode(lxml_child_parent);
176+
retval = LEXBOR_LIBXML2_BRIDGE_STATUS_OOM;
177+
break;
178+
}
179+
174180
lxml_child_parent->parent = lxml_element;
175181
php_dom_add_templated_content(private_data, lxml_element, lxml_child_parent);
176182

‎ext/dom/html_document.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,4 +1644,19 @@ zend_result dom_html_document_title_write(dom_object *obj, zval *newval)
16441644
return SUCCESS;
16451645
}
16461646

1647+
#if ZEND_DEBUG
1648+
PHP_METHOD(Dom_HTMLDocument, debugGetTemplateCount)
1649+
{
1650+
xmlDocPtr doc;
1651+
dom_object *intern;
1652+
1653+
ZEND_PARSE_PARAMETERS_NONE();
1654+
1655+
DOM_GET_OBJ(doc, ZEND_THIS, xmlDocPtr, intern);
1656+
ZEND_IGNORE_VALUE(doc);
1657+
1658+
RETURN_LONG(php_dom_get_template_count((const php_dom_private_data *) intern->document->private_data));
1659+
}
1660+
#endif
1661+
16471662
#endif /* HAVE_LIBXML && HAVE_DOM */

‎ext/dom/php_dom.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,15 +1395,6 @@ void dom_objects_free_storage(zend_object *object)
13951395
xmlNodePtr node = ptr->node;
13961396

13971397
if (node->type != XML_DOCUMENT_NODE && node->type != XML_HTML_DOCUMENT_NODE) {
1398-
/* Destroy associated template content. */
1399-
php_dom_private_data *private_data;
1400-
if (node->type == XML_ELEMENT_NODE
1401-
&& ptr->refcount == 1
1402-
&& intern->document != NULL
1403-
&& (private_data = php_dom_get_private_data(intern))) {
1404-
php_dom_remove_templated_content(private_data, node);
1405-
}
1406-
14071398
php_libxml_node_decrement_resource((php_libxml_node_object *) intern);
14081399
} else {
14091400
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
@@ -2707,4 +2698,22 @@ xmlDocPtr php_dom_create_html_doc(void)
27072698
return lxml_doc;
27082699
}
27092700

2701+
zend_result dom_add_element_destruct_callback_marker(xmlNodePtr element, php_dom_private_data *private_data)
2702+
{
2703+
xmlNsPtr ns = xmlMalloc(sizeof(*ns));
2704+
if (UNEXPECTED(ns == NULL)) {
2705+
return FAILURE;
2706+
}
2707+
2708+
/* Setting the type to XML_ELEMENT_NODE specifies that the namespace definition is actually a special marker
2709+
* for the libxml extension. This is possible because `_private` can be anything, this is a valid definition,
2710+
* and we control the destruction. */
2711+
memset(ns, 0, sizeof(*ns));
2712+
ns->type = XML_ELEMENT_NODE;
2713+
ns->_private = private_data;
2714+
element->nsDef = ns;
2715+
2716+
return SUCCESS;
2717+
}
2718+
27102719
#endif /* HAVE_DOM */

‎ext/dom/php_dom.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ const char *dom_locate_a_namespace(const xmlNode *node, const zend_string *prefi
179179
void dom_mark_namespaces_as_attributes_too(php_dom_libxml_ns_mapper *ns_mapper, xmlDocPtr doc);
180180
bool dom_compare_value(const xmlAttr *attr, const xmlChar *value);
181181
void dom_attr_value_will_change(dom_object *obj, xmlAttrPtr attrp);
182+
zend_result dom_add_element_destruct_callback_marker(xmlNodePtr element, php_dom_private_data *private_data);
182183

183184
typedef enum {
184185
DOM_LOAD_STRING = 0,

‎ext/dom/php_dom.stub.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,6 +1643,10 @@ public function saveXmlFile(string $filename, int $options = 0): int|false {}
16431643
public function saveHtml(?Node $node = null): string {}
16441644

16451645
public function saveHtmlFile(string $filename): int|false {}
1646+
1647+
#if ZEND_DEBUG
1648+
public function debugGetTemplateCount(): int {}
1649+
#endif
16461650
}
16471651

16481652
final class XMLDocument extends Document

‎ext/dom/php_dom_arginfo.h

Lines changed: 12 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/dom/private_data.c

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ static void php_dom_libxml_private_data_destroy(php_libxml_private_data_header *
3535
php_dom_private_data_destroy((php_dom_private_data *) header);
3636
}
3737

38+
static void php_dom_libxml_private_data_free_element(php_libxml_private_data_header *header, xmlNodePtr node)
39+
{
40+
php_dom_remove_templated_content((php_dom_private_data *) header, node);
41+
}
42+
3843
PHP_DOM_EXPORT php_libxml_private_data_header *php_dom_libxml_private_data_header(php_dom_private_data *private_data)
3944
{
4045
return private_data == NULL ? NULL : &private_data->header;
@@ -47,19 +52,24 @@ PHP_DOM_EXPORT php_dom_libxml_ns_mapper *php_dom_ns_mapper_from_private(php_dom_
4752

4853
PHP_DOM_EXPORT php_dom_private_data *php_dom_private_data_create(void)
4954
{
50-
php_dom_private_data *mapper = emalloc(sizeof(*mapper));
51-
mapper->header.dtor = php_dom_libxml_private_data_destroy;
52-
mapper->ns_mapper.html_ns = NULL;
53-
mapper->ns_mapper.prefixless_xmlns_ns = NULL;
54-
zend_hash_init(&mapper->ns_mapper.uri_to_prefix_map, 0, NULL, ZVAL_PTR_DTOR, false);
55-
mapper->template_fragments = NULL;
56-
return mapper;
55+
php_dom_private_data *private_data = emalloc(sizeof(*private_data));
56+
private_data->header.dtor = php_dom_libxml_private_data_destroy;
57+
private_data->header.free_element = php_dom_libxml_private_data_free_element;
58+
private_data->ns_mapper.html_ns = NULL;
59+
private_data->ns_mapper.prefixless_xmlns_ns = NULL;
60+
zend_hash_init(&private_data->ns_mapper.uri_to_prefix_map, 0, NULL, ZVAL_PTR_DTOR, false);
61+
private_data->template_fragments = NULL;
62+
return private_data;
5763
}
5864

5965
void php_dom_private_data_destroy(php_dom_private_data *data)
6066
{
6167
zend_hash_destroy(&data->ns_mapper.uri_to_prefix_map);
6268
if (data->template_fragments != NULL) {
69+
xmlNodePtr node;
70+
ZEND_HASH_MAP_FOREACH_PTR(data->template_fragments, node) {
71+
xmlFreeNode(node);
72+
} ZEND_HASH_FOREACH_END();
6373
zend_hash_destroy(data->template_fragments);
6474
FREE_HASHTABLE(data->template_fragments);
6575
}
@@ -114,6 +124,10 @@ xmlNodePtr php_dom_ensure_templated_content(php_dom_private_data *private_data,
114124
result = xmlNewDocFragment(template_node->doc);
115125
if (EXPECTED(result != NULL)) {
116126
result->parent = template_node;
127+
if (UNEXPECTED(dom_add_element_destruct_callback_marker(template_node, private_data) != SUCCESS)) {
128+
xmlFreeNode(result);
129+
return NULL;
130+
}
117131
php_dom_add_templated_content(private_data, template_node, result);
118132
}
119133
}
@@ -137,4 +151,13 @@ void php_dom_remove_templated_content(php_dom_private_data *private_data, const
137151
}
138152
}
139153

154+
zend_long php_dom_get_template_count(const php_dom_private_data *private_data)
155+
{
156+
if (private_data->template_fragments != NULL) {
157+
return zend_hash_num_elements(private_data->template_fragments);
158+
} else {
159+
return 0;
160+
}
161+
}
162+
140163
#endif /* HAVE_LIBXML && HAVE_DOM */

‎ext/dom/private_data.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,6 @@ void php_dom_add_templated_content(php_dom_private_data *private_data, const xml
4444
xmlNodePtr php_dom_retrieve_templated_content(php_dom_private_data *private_data, const xmlNode *template_node);
4545
xmlNodePtr php_dom_ensure_templated_content(php_dom_private_data *private_data, xmlNodePtr template_node);
4646
void php_dom_remove_templated_content(php_dom_private_data *private_data, const xmlNode *template_node);
47+
zend_long php_dom_get_template_count(const php_dom_private_data *private_data);
4748

4849
#endif

‎ext/dom/tests/modern/common/template_simplexml.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
SimpleXML and template content
3+
--EXTENSIONS--
4+
dom
5+
simplexml
6+
--FILE--
7+
<?php
8+
$dom = Dom\HTMLDocument::createFromString('<template>foo<template>nested</template></template>', LIBXML_NOERROR);
9+
$head = $dom->head;
10+
$head_sxe = simplexml_import_dom($head);
11+
var_dump($head_sxe);
12+
var_dump($dom->debugGetTemplateCount());
13+
unset($head_sxe->template);
14+
var_dump($head_sxe);
15+
var_dump($dom->debugGetTemplateCount());
16+
?>
17+
--EXPECT--

‎ext/libxml/libxml.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,18 @@ static void php_libxml_node_free(xmlNodePtr node)
298298
* very often (why would you remove the whole document?).
299299
*/
300300
xmlNsPtr ns = node->nsDef;
301-
xmlNsPtr last = ns;
302-
while (last->next) {
303-
last = last->next;
301+
if (ns->type == XML_ELEMENT_NODE) {
302+
/* Special marker */
303+
php_libxml_private_data_header *header = ns->_private;
304+
header->free_element(header, node);
305+
xmlFree(ns);
306+
} else {
307+
xmlNsPtr last = ns;
308+
while (last->next) {
309+
last = last->next;
310+
}
311+
php_libxml_set_old_ns_list(node->doc, ns, last);
304312
}
305-
php_libxml_set_old_ns_list(node->doc, ns, last);
306313
node->nsDef = NULL;
307314
}
308315
xmlFreeNode(node);

‎ext/libxml/php_libxml.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ typedef struct {
6767

6868
typedef struct php_libxml_private_data_header {
6969
void (*dtor)(struct php_libxml_private_data_header *);
70+
void (*free_element)(struct php_libxml_private_data_header *, xmlNodePtr);
7071
/* extra fields */
7172
} php_libxml_private_data_header;
7273

0 commit comments

Comments
 (0)