[php-src] master: dom: invalidate node list caches on class attribute mutations
Ilia Alshanetsky <[email protected]>
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: Ilia Alshanetsky (iliaal)
Date: 2026-08-30T11:55:18-04:00
Commit: https://github.com/php/php-src/commit/fbca6cb2219353420490aa746151d0660a4037bc
Raw diff: https://github.com/php/php-src/commit/fbca6cb2219353420490aa746151d0660a4037bc.diff
dom: invalidate node list caches on class attribute mutations
Reflected attribute writes such as className and id, classList
mutations, and removeAttribute()/removeAttributeNS()/removeAttributeNode()
modified attributes without bumping the document cache tag, so live
HTMLCollection caches like getElementsByClassName() kept serving stale
lengths and items. Invalidate the node list caches at every one of these
mutation points. Sibling audit: Attr:: writes, setAttribute(),
setAttributeNode() and setAttributeNS() already invalidate;
php_dom_ns_compat_mark_attribute() only mirrors namespace declarations
during reconciliation and is not user-visible.
Closes GH-23501
Changed paths:
A ext/dom/tests/modern/common/getElementsByClassName_cache_invalidation.phpt
M NEWS
M ext/dom/element.c
M ext/dom/token_list.c
Diff:
diff --git a/NEWS b/NEWS
index ccb18766e2ae..693ff78fafd0 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,8 @@ PHP NEWS
middle generator delegates again). (Lazizbek Ergashev)
- DOM:
+ . Fixed stale getElementsByClassName() and other node list caches after
+ className/classList writes and attribute removals. (Ilia Alshanetsky)
. Fixed a use-after-free when cloning a DOMNameSpaceNode after
DOMDocument::xinclude(). (iliaal)
. Fixed a crash in DOMXPath when a php:function callback receives a nodeset
diff --git a/ext/dom/element.c b/ext/dom/element.c
index 2320216f8244..8dd7f3756050 100644
--- a/ext/dom/element.c
+++ b/ext/dom/element.c
@@ -156,6 +156,7 @@ static xmlAttrPtr dom_element_reflected_attribute_write(dom_object *obj, zval *n
/* Typed property, so it is a string already */
ZEND_ASSERT(Z_TYPE_P(newval) == IS_STRING);
+ php_libxml_invalidate_node_list_cache(obj->document);
return xmlSetNsProp(nodep, NULL, (const xmlChar *) name, (const xmlChar *) Z_STRVAL_P(newval));
}
@@ -544,7 +545,7 @@ static void dom_deep_ns_redef(xmlNodePtr node, xmlNsPtr ns_to_redefine)
efree(worklist);
}
-static bool dom_remove_attribute(xmlNodePtr thisp, xmlNodePtr attrp)
+static bool dom_remove_attribute(xmlNodePtr thisp, xmlNodePtr attrp, php_libxml_ref_obj *document)
{
ZEND_ASSERT(thisp != NULL);
ZEND_ASSERT(attrp != NULL);
@@ -599,6 +600,7 @@ static bool dom_remove_attribute(xmlNodePtr thisp, xmlNodePtr attrp)
return false;
EMPTY_SWITCH_DEFAULT_CASE();
}
+ php_libxml_invalidate_node_list_cache(document);
return true;
}
@@ -624,7 +626,7 @@ PHP_METHOD(DOMElement, removeAttribute)
RETURN_FALSE;
}
- RETURN_BOOL(dom_remove_attribute(nodep, attrp));
+ RETURN_BOOL(dom_remove_attribute(nodep, attrp, intern->document));
}
PHP_METHOD(Dom_Element, removeAttribute)
@@ -642,7 +644,7 @@ PHP_METHOD(Dom_Element, removeAttribute)
attrp = dom_get_attribute_or_nsdecl(intern, nodep, BAD_CAST name, name_len);
if (attrp != NULL) {
- dom_remove_attribute(nodep, attrp);
+ dom_remove_attribute(nodep, attrp, intern->document);
}
}
/* }}} end dom_element_remove_attribute */
@@ -800,6 +802,7 @@ static void dom_element_remove_attribute_node(INTERNAL_FUNCTION_PARAMETERS, zend
RETURN_FALSE;
}
+ php_libxml_invalidate_node_list_cache(intern->document);
xmlUnlinkNode((xmlNodePtr) attrp);
DOM_RET_OBJ((xmlNodePtr) attrp, intern);
@@ -1200,6 +1203,7 @@ PHP_METHOD(DOMElement, removeAttributeNS)
if (nsptr != NULL) {
if (xmlStrEqual(BAD_CAST uri, nsptr->href)) {
dom_eliminate_ns(nodep, nsptr);
+ php_libxml_invalidate_node_list_cache(intern->document);
} else {
return;
}
@@ -1214,6 +1218,7 @@ PHP_METHOD(DOMElement, removeAttributeNS)
} else {
xmlUnlinkNode((xmlNodePtr) attrp);
}
+ php_libxml_invalidate_node_list_cache(intern->document);
}
}
/* }}} end dom_element_remove_attribute_ns */
@@ -1922,7 +1927,7 @@ PHP_METHOD(DOMElement, toggleAttribute)
/* Step 5 */
if (force_is_null || !force) {
- retval = !dom_remove_attribute(thisp, attribute);
+ retval = !dom_remove_attribute(thisp, attribute, intern->document);
goto out;
}
diff --git a/ext/dom/tests/modern/common/getElementsByClassName_cache_invalidation.phpt b/ext/dom/tests/modern/common/getElementsByClassName_cache_invalidation.phpt
new file mode 100644
index 000000000000..4efdad1b59b4
--- /dev/null
+++ b/ext/dom/tests/modern/common/getElementsByClassName_cache_invalidation.phpt
@@ -0,0 +1,44 @@
+--TEST--
+getElementsByClassName() cache must be invalidated by class attribute mutations
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+function mk($body) {
+ return Dom\HTMLDocument::createFromString("<!DOCTYPE html><html><body>$body</body></html>");
+}
+
+$checks = [
+ 'className' => function ($doc, $span) { $span->className = 'zzz'; },
+ 'classList-remove' => function ($doc, $span) { $span->classList->remove('foo'); },
+ 'classList-value' => function ($doc, $span) { $span->classList->value = 'zzz'; },
+ 'setAttribute' => function ($doc, $span) { $span->setAttribute('class', 'zzz'); },
+ 'removeAttribute' => function ($doc, $span) { $span->removeAttribute('class'); },
+ 'removeAttributeNode' => function ($doc, $span) { $span->removeAttributeNode($span->attributes['class']); },
+];
+foreach ($checks as $label => $fn) {
+ $doc = mk('<span class="foo"></span>');
+ $coll = $doc->getElementsByClassName('foo');
+ if ($coll->length !== 1) {
+ echo "$label: unexpected initial length\n";
+ continue;
+ }
+ $fn($doc, $doc->querySelector('span'));
+ echo "$label: ", $coll->length === 0 ? "OK" : "STALE {$coll->length}", "\n";
+}
+
+$doc = mk('<span></span>');
+$coll = $doc->getElementsByClassName('foo');
+var_dump($coll->length);
+$doc->querySelector('span')->className = 'foo';
+echo $coll->length === 1 ? "growth OK" : "growth STALE", "\n";
+?>
+--EXPECT--
+className: OK
+classList-remove: OK
+classList-value: OK
+setAttribute: OK
+removeAttribute: OK
+removeAttributeNode: OK
+int(0)
+growth OK
diff --git a/ext/dom/token_list.c b/ext/dom/token_list.c
index 524ff699f41b..34e2aa6b3ca3 100644
--- a/ext/dom/token_list.c
+++ b/ext/dom/token_list.c
@@ -184,6 +184,7 @@ static void dom_token_list_update(dom_token_list_object *intern)
HashTable *token_set = TOKEN_LIST_GET_SET(intern);
php_libxml_invalidate_cache_tag(&intern->cache_tag);
+ php_libxml_invalidate_node_list_cache(intern->dom.document);
/* 1. If the associated element does not have an associated attribute and token set is empty, then return. */
if (attr == NULL && zend_hash_num_elements(token_set) == 0) {
@@ -432,6 +433,7 @@ zend_result dom_token_list_value_write(dom_object *obj, zval *newval)
zend_value_error("Value must not contain any null bytes");
return FAILURE;
}
+ php_libxml_invalidate_node_list_cache(intern->dom.document);
xmlSetNsProp(dom_token_list_get_element(intern), NULL, BAD_CAST "class", BAD_CAST Z_STRVAL_P(newval));
/* Note: we don't update the set here, the set is always lazily updated for performance reasons. */
return SUCCESS;