Skip to content

Commit 111072a

Browse files
committed
Fix GH-18744: PHP 8.4 classList works not correctly if copy HTMLElement by clone keyword.
The $classList property is special in the sense that it's a cached object instance per (HTML)Element instance. The reason for this design is because it has the [[SameObject]] IDL attribute. Cloning in PHP also clones the properties, so it also clones the cached instance. To solve this, we undo this by resetting the backing storage. Closes GH-18749.
1 parent 87ff547 commit 111072a

File tree

5 files changed

+66
-7
lines changed

5 files changed

+66
-7
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ PHP NEWS
1919
- Date:
2020
. Fix leaks with multiple calls to DatePeriod iterator current(). (nielsdos)
2121

22+
- DOM:
23+
. Fixed bug GH-18744 (classList works not correctly if copy HTMLElement by
24+
clone keyword). (nielsdos)
25+
2226
- FPM:
2327
. Fixed GH-18662 (fpm_get_status segfault). (txuna)
2428

ext/dom/element.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,7 @@ zend_result dom_element_class_name_write(dom_object *obj, zval *newval)
177177
}
178178
/* }}} */
179179

180-
/* {{{ classList TokenList
181-
URL: https://dom.spec.whatwg.org/#dom-element-classlist
182-
*/
183-
zend_result dom_element_class_list_read(dom_object *obj, zval *retval)
180+
zval *dom_element_class_list_zval(dom_object *obj)
184181
{
185182
const uint32_t PROP_INDEX = 0;
186183

@@ -191,7 +188,15 @@ zend_result dom_element_class_list_read(dom_object *obj, zval *retval)
191188
ZEND_ASSERT(OBJ_PROP_TO_NUM(prop_info->offset) == PROP_INDEX);
192189
#endif
193190

194-
zval *cached_token_list = OBJ_PROP_NUM(&obj->std, PROP_INDEX);
191+
return OBJ_PROP_NUM(&obj->std, PROP_INDEX);
192+
}
193+
194+
/* {{{ classList TokenList
195+
URL: https://dom.spec.whatwg.org/#dom-element-classlist
196+
*/
197+
zend_result dom_element_class_list_read(dom_object *obj, zval *retval)
198+
{
199+
zval *cached_token_list = dom_element_class_list_zval(obj);
195200
if (Z_ISUNDEF_P(cached_token_list)) {
196201
object_init_ex(cached_token_list, dom_token_list_class_entry);
197202
dom_token_list_object *intern = php_dom_token_list_from_obj(Z_OBJ_P(cached_token_list));

ext/dom/php_dom.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ static zend_object_handlers dom_modern_nodelist_object_handlers;
101101
static zend_object_handlers dom_html_collection_object_handlers;
102102
static zend_object_handlers dom_object_namespace_node_handlers;
103103
static zend_object_handlers dom_modern_domimplementation_object_handlers;
104+
static zend_object_handlers dom_modern_element_object_handlers;
104105
static zend_object_handlers dom_token_list_object_handlers;
105106
#ifdef LIBXML_XPATH_ENABLED
106107
zend_object_handlers dom_xpath_object_handlers;
@@ -669,6 +670,21 @@ static zend_object *dom_objects_store_clone_obj(zend_object *zobject) /* {{{ */
669670
}
670671
/* }}} */
671672

673+
static zend_object *dom_modern_element_clone_obj(zend_object *zobject)
674+
{
675+
zend_object *clone = dom_objects_store_clone_obj(zobject);
676+
677+
/* The $classList property is unique per element, and cached due to its [[SameObject]] requirement.
678+
* Remove it from the clone so the clone will get a fresh instance upon demand. */
679+
zval *class_list = dom_element_class_list_zval(php_dom_obj_from_obj(clone));
680+
if (!Z_ISUNDEF_P(class_list)) {
681+
zval_ptr_dtor(class_list);
682+
ZVAL_UNDEF(class_list);
683+
}
684+
685+
return clone;
686+
}
687+
672688
static zend_object *dom_object_namespace_node_clone_obj(zend_object *zobject)
673689
{
674690
dom_object_namespace_node *intern = php_dom_namespace_node_obj_from_obj(zobject);
@@ -778,6 +794,9 @@ PHP_MINIT_FUNCTION(dom)
778794
* one instance per parent object. */
779795
dom_modern_domimplementation_object_handlers.clone_obj = NULL;
780796

797+
memcpy(&dom_modern_element_object_handlers, &dom_object_handlers, sizeof(zend_object_handlers));
798+
dom_modern_element_object_handlers.clone_obj = dom_modern_element_clone_obj;
799+
781800
memcpy(&dom_nnodemap_object_handlers, &dom_object_handlers, sizeof(zend_object_handlers));
782801
dom_nnodemap_object_handlers.free_obj = dom_nnodemap_objects_free_storage;
783802
dom_nnodemap_object_handlers.read_dimension = dom_nodemap_read_dimension;
@@ -1108,7 +1127,7 @@ PHP_MINIT_FUNCTION(dom)
11081127

11091128
dom_modern_element_class_entry = register_class_Dom_Element(dom_modern_node_class_entry, dom_modern_parentnode_class_entry, dom_modern_childnode_class_entry);
11101129
dom_modern_element_class_entry->create_object = dom_objects_new;
1111-
dom_modern_element_class_entry->default_object_handlers = &dom_object_handlers;
1130+
dom_modern_element_class_entry->default_object_handlers = &dom_modern_element_object_handlers;
11121131

11131132
zend_hash_init(&dom_modern_element_prop_handlers, 0, NULL, NULL, true);
11141133
DOM_REGISTER_PROP_HANDLER(&dom_modern_element_prop_handlers, "namespaceURI", dom_node_namespace_uri_read, NULL);
@@ -1132,7 +1151,7 @@ PHP_MINIT_FUNCTION(dom)
11321151

11331152
dom_html_element_class_entry = register_class_Dom_HTMLElement(dom_modern_element_class_entry);
11341153
dom_html_element_class_entry->create_object = dom_objects_new;
1135-
dom_html_element_class_entry->default_object_handlers = &dom_object_handlers;
1154+
dom_html_element_class_entry->default_object_handlers = &dom_modern_element_object_handlers;
11361155
zend_hash_add_new_ptr(&classes, dom_html_element_class_entry->name, &dom_modern_element_prop_handlers);
11371156

11381157
dom_text_class_entry = register_class_DOMText(dom_characterdata_class_entry);

ext/dom/php_dom.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ bool php_dom_create_nullable_object(xmlNodePtr obj, zval *return_value, dom_obje
187187
xmlNodePtr dom_clone_node(php_dom_libxml_ns_mapper *ns_mapper, xmlNodePtr node, xmlDocPtr doc, bool recursive);
188188
void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document);
189189
void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document);
190+
zval *dom_element_class_list_zval(dom_object *obj);
190191

191192
typedef enum {
192193
DOM_LOAD_STRING = 0,
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
GH-18744 (classList works not correctly if copy HTMLElement by clone keyword.)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$doc = \Dom\HTMLDocument::createEmpty();
9+
$ele1 = $doc->createElement('div');
10+
$ele1->classList->add('foo');
11+
$ele2 = clone $ele1;
12+
$ele2->classList->add('bar');
13+
14+
echo "Element1 class: " . $ele1->getAttribute('class');
15+
echo "\n";
16+
echo "Element2 class: " . $ele2->getAttribute('class');
17+
echo "\n";
18+
19+
var_dump($ele1->classList !== $ele2->classList);
20+
// These comparisons are not pointless: they're getters and should not create new objects
21+
var_dump($ele1->classList === $ele1->classList);
22+
var_dump($ele2->classList === $ele2->classList);
23+
24+
?>
25+
--EXPECT--
26+
Element1 class: foo
27+
Element2 class: foo bar
28+
bool(true)
29+
bool(true)
30+
bool(true)

0 commit comments

Comments
 (0)