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: