diff --git a/NEWS b/NEWS index a5d2cfe2c037..b58cfbce4355 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. (iliaal) . 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-- +
$body"); +} + +$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(''); + $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(''); +$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;