From a252473e005901a1bec18ccf834f8a202dcdb473 Mon Sep 17 00:00:00 2001 From: NathanFreeman <1056159381@qq.com> Date: Thu, 23 Feb 2023 22:29:27 +0800 Subject: [PATCH 1/3] Fix bug 80602 --- NEWS | 3 + ext/dom/parentnode.c | 60 ++++++++++-- ext/dom/tests/bug80602.phpt | 178 ++++++++++++++++++++++++++++++++++ ext/dom/tests/bug80602_2.phpt | 178 ++++++++++++++++++++++++++++++++++ 4 files changed, 410 insertions(+), 9 deletions(-) create mode 100644 ext/dom/tests/bug80602.phpt create mode 100644 ext/dom/tests/bug80602_2.phpt diff --git a/NEWS b/NEWS index 3ee934bca5839..c7d4d4bed0295 100644 --- a/NEWS +++ b/NEWS @@ -948,6 +948,9 @@ PHP NEWS . Fixed bug #81433 (DOMElement::setIdAttribute() called twice may remove ID). (Viktor Volkov) + . Fixed bug #80602 (Segfault when using DOMChildNode::before()). + (Nathan Freeman) + - FFI: . Fixed bug #79576 ("TYPE *" shows unhelpful message when type is not defined). (Dmitry) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index 3d7e5e44a4b2c..1a210bfb487ac 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -181,6 +181,10 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod return NULL; } + if (nodesc > 1) { + newNode = xmlCopyNode(newNode, 1); + } + if (!xmlAddChild(fragment, newNode)) { xmlFree(fragment); @@ -302,7 +306,9 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) { xmlNode *prevsib = dom_object_get_node(context); xmlNodePtr newchild, parentNode; - xmlNode *fragment; + xmlNode *fragment, *nextsib; + xmlDoc *doc; + bool afterlastchild; int stricterror = dom_get_strict_error(context->document); @@ -311,7 +317,10 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) return; } + doc = prevsib->doc; parentNode = prevsib->parent; + nextsib = prevsib->next; + afterlastchild = (nextsib == NULL); fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -321,13 +330,31 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { - fragment->last->next = prevsib->next; - prevsib->next = newchild; + if (!parentNode->children) { + prevsib = nextsib = NULL; + } else if (afterlastchild) { + prevsib = parentNode->children == prevsib ? prevsib : parentNode->last; + } else { + prevsib = parentNode->children == prevsib ? prevsib : NULL; + } - newchild->prev = prevsib; + if (prevsib) { + fragment->last->next = prevsib->next; + if (prevsib->next) { + prevsib->next->prev = fragment->last; + } + prevsib->next = newchild; + } else { + parentNode->children = newchild; + if (nextsib) { + fragment->last->next = nextsib; + nextsib->prev = fragment->last; + } + } + newchild->prev = prevsib; dom_fragment_assign_parent_node(parentNode, fragment); - dom_reconcile_ns(prevsib->doc, newchild); + dom_reconcile_ns(doc, newchild); } xmlFree(fragment); @@ -337,10 +364,15 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) { xmlNode *nextsib = dom_object_get_node(context); xmlNodePtr newchild, prevsib, parentNode; - xmlNode *fragment; + xmlNode *fragment, *afternextsib; + xmlDoc *doc; + bool beforefirstchild; + doc = nextsib->doc; prevsib = nextsib->prev; + afternextsib = nextsib->next; parentNode = nextsib->parent; + beforefirstchild = !prevsib; fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -350,19 +382,29 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { + if (!parentNode->children) { + nextsib = NULL; + } else if (beforefirstchild) { + nextsib = parentNode->children == nextsib ? nextsib : afternextsib; + } else { + nextsib = parentNode->children == prevsib ? prevsib->next : nextsib; + } + if (parentNode->children == nextsib) { parentNode->children = newchild; } else { prevsib->next = newchild; } + fragment->last->next = nextsib; - nextsib->prev = fragment->last; + if (nextsib) { + nextsib->prev = fragment->last; + } newchild->prev = prevsib; dom_fragment_assign_parent_node(parentNode, fragment); - - dom_reconcile_ns(nextsib->doc, newchild); + dom_reconcile_ns(doc, newchild); } xmlFree(fragment); diff --git a/ext/dom/tests/bug80602.phpt b/ext/dom/tests/bug80602.phpt new file mode 100644 index 0000000000000..9f041f686f516 --- /dev/null +++ b/ext/dom/tests/bug80602.phpt @@ -0,0 +1,178 @@ +--TEST-- +Bug #80602 (Segfault when using DOMChildNode::before()) +--FILE-- +loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($doc->documentElement->firstChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($target, $doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($doc->documentElement->lastChild, $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($target, $doc->documentElement->firstChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($doc->documentElement->firstChild, $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before('bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before('bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($target, 'bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before('bar', $target, 'baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before('bar', 'baz', $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($target, 'bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before('bar', $target, 'baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before('bar', 'baz', $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before('bar', $target, $doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($target, 'bar', $doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->before($target, $doc->documentElement->lastChild, 'bar'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before('bar', $doc->documentElement->firstChild, $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($doc->documentElement->firstChild, 'bar', $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before($doc->documentElement->firstChild, $target, 'bar'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +?> +--EXPECTF-- +foo +foo +foo +foo +foo +foo +foo +foo +barbazfoo +foobarbaz +foobarbaz +barfoobaz +barbazfoo +foobarbaz +foobarbaz +foobarbaz +barfoo +foobar +foobar +barfoo +foobar +foobar diff --git a/ext/dom/tests/bug80602_2.phpt b/ext/dom/tests/bug80602_2.phpt new file mode 100644 index 0000000000000..1151417c0f845 --- /dev/null +++ b/ext/dom/tests/bug80602_2.phpt @@ -0,0 +1,178 @@ +--TEST-- +Bug #80602 (Segfault when using DOMChildNode::after()) +--FILE-- +loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($doc->documentElement->firstChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($target, $doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($doc->documentElement->lastChild, $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($target, $doc->documentElement->firstChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($doc->documentElement->firstChild, $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after('bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after('bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($target, 'bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after('bar', $target, 'baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after('bar', 'baz', $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($target, 'bar','baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after('bar', $target, 'baz'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after('bar', 'baz', $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after('bar', $target, $doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($target, 'bar', $doc->documentElement->lastChild); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->firstChild; +$target->after($target, $doc->documentElement->lastChild, 'bar'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after('bar', $doc->documentElement->firstChild, $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($doc->documentElement->firstChild, 'bar', $target); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->after($doc->documentElement->firstChild, $target, 'bar'); +echo $doc->saveXML($doc->documentElement).PHP_EOL; + +?> +--EXPECTF-- +foo +foo +foo +foo +foo +foo +foo +foo +foobarbaz +foobarbaz +foobarbaz +barfoobaz +barbazfoo +foobarbaz +foobarbaz +foobarbaz +barfoo +foobar +foobar +barfoo +foobar +foobar From 886d60c8326f73223fb7d5ef216e3833a7764147 Mon Sep 17 00:00:00 2001 From: NathanFreeman <1056159381@qq.com> Date: Sun, 26 Mar 2023 21:19:38 +0800 Subject: [PATCH 2/3] add comment --- ext/dom/parentnode.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index 1a210bfb487ac..429dff5730049 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -181,6 +181,11 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod return NULL; } + /* + * xmlNewDocText function will always returns same address to the second parameter if the parameters are greater than or equal to three. + * If it's text, that's fine, but if it's an object, it can cause invalid pointer because many new nodes point to the same memory address. + * So we must copy the new node to avoid this situation. + */ if (nodesc > 1) { newNode = xmlCopyNode(newNode, 1); } @@ -330,11 +335,22 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { + // first node and last node are both both parameters to DOMElement::before() method so nextsib and prevsib are null. if (!parentNode->children) { prevsib = nextsib = NULL; } else if (afterlastchild) { + /* + * The new node will be inserted after last node, prevsib is last node. + * The first node is the parameter to DOMElement::after() if parentNode->children == prevsib is true + * and prevsib does not change, otherwise prevsib is parentNode->last(first mode). + */ prevsib = parentNode->children == prevsib ? prevsib : parentNode->last; } else { + /* + * The new node will be inserted after first node, prevsib is first node. + * The first node is not the parameter to DOMElement::after() if parentNode->children == prevsib is true + * and prevsib does not change otherwise prevsib is null to mean that parentNode->children is the new node. + */ prevsib = parentNode->children == prevsib ? prevsib : NULL; } @@ -382,11 +398,22 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { + // first node and last node are both both parameters to DOMElement::before() method so nextsib is null. if (!parentNode->children) { nextsib = NULL; } else if (beforefirstchild) { + /* + * The new node will be inserted before first node, nextsib is first node and afternextsib is last node. + * The first node is not the parameter to DOMElement::before() if parentNode->children == nextsib is true + * and nextsib does not change, otherwise nextsib is the last node. + */ nextsib = parentNode->children == nextsib ? nextsib : afternextsib; } else { + /* + * The new node will be inserted before last node, prevsib is first node and nestsib is last node. + * The first node is not the parameter to DOMElement::before() if parentNode->children == prevsib is true + * but last node may be, so use prevsib->next to determine the value of nextsib, otherwise nextsib does not change. + */ nextsib = parentNode->children == prevsib ? prevsib->next : nextsib; } From 476de00e06b972bec29d7c6757126a9d1643cbff Mon Sep 17 00:00:00 2001 From: NathanFreeman <1056159381@qq.com> Date: Sun, 26 Mar 2023 21:22:15 +0800 Subject: [PATCH 3/3] fix comment --- ext/dom/parentnode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index 429dff5730049..2352624d0f4d7 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -335,7 +335,7 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) newchild = fragment->children; if (newchild) { - // first node and last node are both both parameters to DOMElement::before() method so nextsib and prevsib are null. + // first node and last node are both both parameters to DOMElement::after() method so nextsib and prevsib are null. if (!parentNode->children) { prevsib = nextsib = NULL; } else if (afterlastchild) {