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--
+', '', 'A'],
+ ['', '', 'A'],
+ ['', '', 'p:A'],
+];
+
+function element(string $attlist, string $root): DOMElement {
+ $doc = new DOMDocument();
+ $doc->loadXML("$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--
+--- ---
+hasAttribute: bool(true)
+getAttribute: string(1) "d"
+setAttribute: DOMAttr
+after setAttribute:
+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:
+--- ---
+hasAttribute: bool(true)
+getAttribute: string(1) "d"
+setAttribute: DOMAttr
+after setAttribute:
+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:
+--- ---
+hasAttribute: bool(true)
+getAttribute: string(1) "d"
+setAttribute: DOMAttr
+after setAttribute:
+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: