From bb4c5f76e3929af2a2505d2591c7e2368c79762a 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 1/2] 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. --- ext/dom/node.c | 19 +++++++++++--- ext/dom/tests/gh10234.phpt | 52 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 ext/dom/tests/gh10234.phpt diff --git a/ext/dom/node.c b/ext/dom/node.c index 880c8cfe3e794..cd92cea2437e6 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 and clears the properties field if needed. */ + 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 0000000000000..f04c4ccdc4d27 --- /dev/null +++ b/ext/dom/tests/gh10234.phpt @@ -0,0 +1,52 @@ +--TEST-- +GH-10234 (Setting DOMAttr::textContent results in an empty attribute value.) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$attribute = $document->documentElement->getAttributeNode('attribute'); + +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()); + +$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()); +?> +--EXPECT-- +string(38) " +" +string(5) "value" +string(9) "new value" +string(42) " +" +string(13) "hello & world" +string(50) " +" +string(9) "hi" +string(54) " +" +string(13) "hello & world" +string(71) "hello & world +" +string(9) "hi" +string(75) "<b>hi</b> +" From 33dc3693fb788ddd6c8d51d39d6b996c5a77ad2c Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Sun, 28 May 2023 00:37:46 +0200 Subject: [PATCH 2/2] Test one additional case --- ext/dom/tests/gh10234.phpt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ext/dom/tests/gh10234.phpt b/ext/dom/tests/gh10234.phpt index f04c4ccdc4d27..63571edac3160 100644 --- a/ext/dom/tests/gh10234.phpt +++ b/ext/dom/tests/gh10234.phpt @@ -30,6 +30,14 @@ 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-- string(38) " @@ -50,3 +58,9 @@ string(71) "hello & worldhi" string(75) "<b>hi</b> " +string(12) "quote "test"" +string(66) "quote "test" +" +string(12) "quote 'test'" +string(66) "quote 'test' +"