From c473787abb486c3f25bd551252b36d617267738f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 6 Jan 2023 19:51:49 +0100 Subject: [PATCH] Fix GH-10234: Setting DOMAttr::textContent results in an empty attribute value We can't directly call xmlNodeSetContent, because it might encode the string through xmlStringLenGetNodeList for types XML_DOCUMENT_FRAG_NODE, XML_ELEMENT_NODE, XML_ATTRIBUTE_NODE. In these cases we need to use a text node to avoid the encoding. For the other cases, we *can* rely on xmlNodeSetContent because it is either a no-op, or handles the content without encoding and clears the properties field if needed. The test was taken from the issue report, for the test: Co-authored-by: ThomasWeinert Closes GH-10245. --- NEWS | 2 + ext/dom/node.c | 19 ++++++-- ext/dom/tests/gh10234.phpt | 93 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 ext/dom/tests/gh10234.phpt diff --git a/NEWS b/NEWS index 3b0d0129954..40fb3e328b7 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,8 @@ PHP NEWS - DOM: . Fixed bugs GH-11288 and GH-11289 and GH-11290 and GH-9142 (DOMExceptions and segfaults with replaceWith). (nielsdos) + . Fixed bug GH-10234 (Setting DOMAttr::textContent results in an empty + attribute value). (nielsdos) - Opcache: . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos) diff --git a/ext/dom/node.c b/ext/dom/node.c index 880c8cfe3e7..b291ccc99a3 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -769,17 +769,28 @@ int dom_node_text_content_write(dom_object *obj, zval *newval) return FAILURE; } - if (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE) { + const xmlChar *xmlChars = (const xmlChar *) ZSTR_VAL(str); + int type = nodep->type; + + /* We can't directly call xmlNodeSetContent, because it might encode the string through + * xmlStringLenGetNodeList for types XML_DOCUMENT_FRAG_NODE, XML_ELEMENT_NODE, XML_ATTRIBUTE_NODE. + * See tree.c:xmlNodeSetContent in libxml. + * In these cases we need to use a text node to avoid the encoding. + * For the other cases, we *can* rely on xmlNodeSetContent because it is either a no-op, or handles + * the content without encoding. */ + if (type == XML_DOCUMENT_FRAG_NODE || type == XML_ELEMENT_NODE || type == XML_ATTRIBUTE_NODE) { if (nodep->children) { node_list_unlink(nodep->children); php_libxml_node_free_list((xmlNodePtr) nodep->children); nodep->children = NULL; } + + xmlNode *textNode = xmlNewText(xmlChars); + xmlAddChild(nodep, textNode); + } else { + xmlNodeSetContent(nodep, xmlChars); } - /* we have to use xmlNodeAddContent() to get the same behavior as with xmlNewText() */ - xmlNodeSetContent(nodep, (xmlChar *) ""); - xmlNodeAddContent(nodep, (xmlChar *) ZSTR_VAL(str)); zend_string_release_ex(str, 0); return SUCCESS; diff --git a/ext/dom/tests/gh10234.phpt b/ext/dom/tests/gh10234.phpt new file mode 100644 index 00000000000..5edc8fc6c1f --- /dev/null +++ b/ext/dom/tests/gh10234.phpt @@ -0,0 +1,93 @@ +--TEST-- +GH-10234 (Setting DOMAttr::textContent results in an empty attribute value.) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$attribute = $document->documentElement->getAttributeNode('attribute'); + +echo "-- Attribute tests --\n"; + +var_dump($document->saveHTML()); +var_dump($attribute->textContent); + +$attribute->textContent = 'new value'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = 'hello & world'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = 'hi'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = 'quote "test"'; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = "quote 'test'"; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +$attribute->textContent = "quote '\"test\"'"; +var_dump($attribute->textContent); +var_dump($document->saveHTML()); + +echo "-- Document element tests --\n"; + +$document->documentElement->textContent = 'hello & world'; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); + +$document->documentElement->textContent = 'hi'; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); + +$document->documentElement->textContent = 'quote "test"'; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); + +$document->documentElement->textContent = "quote 'test'"; +var_dump($document->documentElement->textContent); +var_dump($document->saveHTML()); +?> +--EXPECT-- +-- Attribute tests -- +string(38) " +" +string(5) "value" +string(9) "new value" +string(42) " +" +string(13) "hello & world" +string(50) " +" +string(9) "hi" +string(54) " +" +string(12) "quote "test"" +string(45) " +" +string(12) "quote 'test'" +string(45) " +" +string(14) "quote '"test"'" +string(57) " +" +-- Document element tests -- +string(13) "hello & world" +string(74) "hello & world +" +string(9) "hi" +string(78) "<b>hi</b> +" +string(12) "quote "test"" +string(69) "quote "test" +" +string(12) "quote 'test'" +string(69) "quote 'test' +"