[php-src] PHP-8.5: Fix GH-22825: DOM attribute methods on a DTD default attribute

Author: Ilia Alshanetsky (iliaal)
Date: 2026-07-20T12:59:16-04:00

Commit: https://github.com/php/php-src/commit/145e8c26d4ce147fcb8f2c83f48d920e4d3a37a1
Raw diff: https://github.com/php/php-src/commit/145e8c26d4ce147fcb8f2c83f48d920e4d3a37a1.diff

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

Changed paths:
  A  ext/dom/tests/gh22825.phpt
  M  NEWS
  M  ext/dom/element.c


Diff:

diff --git a/NEWS b/NEWS
index ccae29155dbd..cfb82c4f0ca2 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 e25805df53eb..b2564428db9c 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 000000000000..987ed57a1454
--- /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"/>