Commit 145e8c26d4c for php.net
commit 145e8c26d4ce147fcb8f2c83f48d920e4d3a37a1
Author: Ilia Alshanetsky <ilia@ilia.ws>
Date: Mon Jul 20 08:29:57 2026 -0400
Fix GH-22825: DOM attribute methods on a DTD default attribute
xmlHasNsProp() falls back to the internal subset, so the legacy ext/dom
attribute methods can receive a DTD attribute declaration instead of an
attribute node. setAttribute() and dom_remove_attribute() enumerated only
XML_ATTRIBUTE_NODE and XML_NAMESPACE_DECL, so a defaulted attribute
asserted on debug builds and ran into __builtin_unreachable() on release
builds, and setAttributeNode() skipped the declaration when unlinking the
old attribute but still returned it, throwing after the tree was already
modified. setAttribute() now creates the attribute as it did before 8.4,
matching setAttributeNS(); removeAttribute() reports that nothing was
removed, since the declaration lives in the DTD and not on the element;
setAttributeNode() returns null; and toggleAttribute() derives its result
from the removal instead of assuming it succeeded, so the returned value
keeps agreeing with hasAttribute(). getAttribute() and hasAttribute()
already handled the declaration, and the Dom\Element methods are
unaffected because the spec-compliant lookup never consults the DTD.
Fixes GH-22825
Closes GH-22830
diff --git a/NEWS b/NEWS
index ccae29155db..cfb82c4f0ca 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,10 @@ PHP NEWS
- Date:
. Fixed leak on double DatePeriod::__construct() call. (ilutov)
+- DOM:
+ . Fixed bug GH-22825 (DOMElement::setAttribute() fails silently when the DTD
+ declares a default value for the attribute). (iliaal)
+
- Sockets:
. Fixed various memory related issues in ext/sockets. (David Carlier)
diff --git a/ext/dom/element.c b/ext/dom/element.c
index e25805df53e..b2564428db9 100644
--- a/ext/dom/element.c
+++ b/ext/dom/element.c
@@ -468,6 +468,8 @@ PHP_METHOD(DOMElement, setAttribute)
break;
case XML_NAMESPACE_DECL:
RETURN_FALSE;
+ case XML_ATTRIBUTE_DECL:
+ break;
EMPTY_SWITCH_DEFAULT_CASE();
}
}
@@ -595,6 +597,8 @@ static bool dom_remove_attribute(xmlNodePtr thisp, xmlNodePtr attrp)
break;
}
+ case XML_ATTRIBUTE_DECL:
+ return false;
EMPTY_SWITCH_DEFAULT_CASE();
}
return true;
@@ -726,7 +730,11 @@ static void dom_element_set_attribute_node_common(INTERNAL_FUNCTION_PARAMETERS,
existattrp = xmlHasProp(nodep, attrp->name);
}
- if (existattrp != NULL && existattrp->type != XML_ATTRIBUTE_DECL) {
+ if (existattrp != NULL && existattrp->type == XML_ATTRIBUTE_DECL) {
+ existattrp = NULL;
+ }
+
+ if (existattrp != NULL) {
if ((oldobj = php_dom_object_get_data((xmlNodePtr) existattrp)) != NULL &&
((php_libxml_node_ptr *)oldobj->ptr)->node == (xmlNodePtr) attrp)
{
@@ -1788,8 +1796,7 @@ PHP_METHOD(DOMElement, toggleAttribute)
/* Step 5 */
if (force_is_null || !force) {
- dom_remove_attribute(thisp, attribute);
- retval = false;
+ retval = !dom_remove_attribute(thisp, attribute);
goto out;
}
diff --git a/ext/dom/tests/gh22825.phpt b/ext/dom/tests/gh22825.phpt
new file mode 100644
index 00000000000..987ed57a145
--- /dev/null
+++ b/ext/dom/tests/gh22825.phpt
@@ -0,0 +1,98 @@
+--TEST--
+GH-22825 (DOMElement::setAttribute() reaches EMPTY_SWITCH_DEFAULT_CASE() with DTD #FIXED default attributes)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$cases = [
+ ['<!ATTLIST root A CDATA #FIXED "d">', '<root/>', 'A'],
+ ['<!ATTLIST root A CDATA "d">', '<root/>', 'A'],
+ ['<!ATTLIST root p:A CDATA #FIXED "d">', '<root xmlns:p="urn:p"/>', 'p:A'],
+];
+
+function element(string $attlist, string $root): DOMElement {
+ $doc = new DOMDocument();
+ $doc->loadXML("<!DOCTYPE root [$attlist]>$root");
+ return $doc->documentElement;
+}
+
+foreach ($cases as [$attlist, $root, $name]) {
+ echo "--- $attlist ---\n";
+
+ $el = element($attlist, $root);
+ echo "hasAttribute: ";
+ var_dump($el->hasAttribute($name));
+ echo "getAttribute: ";
+ var_dump($el->getAttribute($name));
+
+ $el = element($attlist, $root);
+ $result = $el->setAttribute($name, 'v');
+ echo "setAttribute: ", is_object($result) ? $result::class : var_export($result, true), "\n";
+ echo "after setAttribute: ", trim($el->ownerDocument->saveXML($el)), "\n";
+
+ $el = element($attlist, $root);
+ echo "removeAttribute: ";
+ var_dump($el->removeAttribute($name));
+ echo "still present after removeAttribute: ";
+ var_dump($el->hasAttribute($name));
+
+ $el = element($attlist, $root);
+ echo "toggleAttribute(false): ";
+ var_dump($el->toggleAttribute($name, false));
+ echo "still present after toggleAttribute(false): ";
+ var_dump($el->hasAttribute($name));
+
+ $el = element($attlist, $root);
+ echo "toggleAttribute(true): ";
+ var_dump($el->toggleAttribute($name, true));
+ echo "still present after toggleAttribute(true): ";
+ var_dump($el->hasAttribute($name));
+
+ $el = element($attlist, $root);
+ $attr = $el->ownerDocument->createAttribute($name);
+ $attr->value = 'z';
+ echo "setAttributeNode: ";
+ var_dump($el->setAttributeNode($attr));
+ echo "after setAttributeNode: ", trim($el->ownerDocument->saveXML($el)), "\n";
+}
+?>
+--EXPECT--
+--- <!ATTLIST root A CDATA #FIXED "d"> ---
+hasAttribute: bool(true)
+getAttribute: string(1) "d"
+setAttribute: DOMAttr
+after setAttribute: <root A="v"/>
+removeAttribute: bool(false)
+still present after removeAttribute: bool(true)
+toggleAttribute(false): bool(true)
+still present after toggleAttribute(false): bool(true)
+toggleAttribute(true): bool(true)
+still present after toggleAttribute(true): bool(true)
+setAttributeNode: NULL
+after setAttributeNode: <root A="z"/>
+--- <!ATTLIST root A CDATA "d"> ---
+hasAttribute: bool(true)
+getAttribute: string(1) "d"
+setAttribute: DOMAttr
+after setAttribute: <root A="v"/>
+removeAttribute: bool(false)
+still present after removeAttribute: bool(true)
+toggleAttribute(false): bool(true)
+still present after toggleAttribute(false): bool(true)
+toggleAttribute(true): bool(true)
+still present after toggleAttribute(true): bool(true)
+setAttributeNode: NULL
+after setAttributeNode: <root A="z"/>
+--- <!ATTLIST root p:A CDATA #FIXED "d"> ---
+hasAttribute: bool(true)
+getAttribute: string(1) "d"
+setAttribute: DOMAttr
+after setAttribute: <root xmlns:p="urn:p" p:A="v"/>
+removeAttribute: bool(false)
+still present after removeAttribute: bool(true)
+toggleAttribute(false): bool(true)
+still present after toggleAttribute(false): bool(true)
+toggleAttribute(true): bool(true)
+still present after toggleAttribute(true): bool(true)
+setAttributeNode: NULL
+after setAttributeNode: <root xmlns:p="urn:p" p:A="z"/>