From 382101eb3d679fd7555ce88d1dea40583b7ed45e Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Mon, 22 May 2023 23:50:06 +0200 Subject: [PATCH 1/3] Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMException with replaceWith This replaces the implementation of before and after with one following the spec very strictly, instead of trying to figure out the state we're in by looking at the pointers. Also relaxes the condition on text node copying to prevent working on a stale node pointer. --- ext/dom/parentnode.c | 179 ++++++++++++++++++---------------- ext/dom/tests/bug80602.phpt | 88 ++++++++--------- ext/dom/tests/bug80602_2.phpt | 88 ++++++++--------- ext/dom/tests/bug80602_3.phpt | 120 +++++++++++++++++++++++ ext/dom/tests/bug80602_4.phpt | 33 +++++++ ext/dom/tests/gh11288.phpt | 79 +++++++++++++++ ext/dom/tests/gh11289.phpt | 34 +++++++ ext/dom/tests/gh11290.phpt | 33 +++++++ ext/dom/tests/gh9142.phpt | 20 ++++ 9 files changed, 504 insertions(+), 170 deletions(-) create mode 100644 ext/dom/tests/bug80602_3.phpt create mode 100644 ext/dom/tests/bug80602_4.phpt create mode 100644 ext/dom/tests/gh11288.phpt create mode 100644 ext/dom/tests/gh11289.phpt create mode 100644 ext/dom/tests/gh11290.phpt create mode 100644 ext/dom/tests/gh9142.phpt diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index cf823057d22ae..c132d6ddc3414 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -124,6 +124,23 @@ int dom_parent_node_child_element_count(dom_object *obj, zval *retval) } /* }}} */ +static bool dom_is_node_in_list(const zval *nodes, int nodesc, const xmlNodePtr node_to_find) +{ + for (int i = 0; i < nodesc; i++) { + if (Z_TYPE(nodes[i]) == IS_OBJECT) { + const zend_class_entry *ce = Z_OBJCE(nodes[i]); + + if (instanceof_function(ce, dom_node_class_entry)) { + if (dom_object_get_node(Z_DOMOBJ_P(nodes + i)) == node_to_find) { + return true; + } + } + } + } + + return false; +} + xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc) { int i; @@ -177,17 +194,16 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod goto hierarchy_request_err; } - /* - * 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) { + /* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild): + * "Add a new node to @parent, at the end of the child (or property) list merging adjacent TEXT nodes (in which case @cur is freed)". + * So we must take a copy if this situation arises to prevent a use-after-free. */ + bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE; + if (will_free) { newNode = xmlCopyNode(newNode, 1); } if (!xmlAddChild(fragment, newNode)) { - if (nodesc > 1) { + if (will_free) { xmlFreeNode(newNode); } goto hierarchy_request_err; @@ -303,25 +319,65 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc) xmlFree(fragment); } +static void dom_pre_insert(xmlNodePtr insertion_point, xmlNodePtr parentNode, xmlNodePtr newchild, xmlNodePtr fragment) +{ + if (!insertion_point) { + /* Place it as last node */ + if (parentNode->children) { + /* There are children */ + fragment->last->prev = parentNode->last; + newchild->prev = parentNode->last->prev; + parentNode->last->next = newchild; + } else { + /* No children, because they moved out when they became a fragment */ + parentNode->children = newchild; + parentNode->last = newchild; + } + } else { + /* Insert fragment before insertion_point */ + fragment->last->next = insertion_point; + if (insertion_point->prev) { + insertion_point->prev->next = newchild; + newchild->prev = insertion_point->prev; + } + insertion_point->prev = newchild; + if (parentNode->children == insertion_point) { + parentNode->children = newchild; + } + } +} + void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) { + /* Spec link: https://dom.spec.whatwg.org/#dom-childnode-after */ + xmlNode *prevsib = dom_object_get_node(context); xmlNodePtr newchild, parentNode; - xmlNode *fragment, *nextsib; + xmlNode *fragment; xmlDoc *doc; - bool afterlastchild; int stricterror = dom_get_strict_error(context->document); - if (!prevsib->parent) { + /* Spec step 1 */ + parentNode = prevsib->parent; + /* Spec step 2 */ + if (!parentNode) { php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror); return; } + /* Spec step 3: find first following child not in nodes; otherwise null */ + xmlNodePtr viable_next_sibling = prevsib->next; + while (viable_next_sibling) { + if (!dom_is_node_in_list(nodes, nodesc, viable_next_sibling)) { + break; + } + viable_next_sibling = viable_next_sibling->next; + } + doc = prevsib->doc; - parentNode = prevsib->parent; - nextsib = prevsib->next; - afterlastchild = (nextsib == NULL); + + /* Spec step 4: convert nodes into fragment */ fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -331,40 +387,9 @@ 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::after() 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 node). - */ - 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; - } - - 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; - } - } + /* Step 5: place fragment into the parent before viable_next_sibling */ + dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment); - newchild->prev = prevsib; dom_fragment_assign_parent_node(parentNode, fragment); dom_reconcile_ns(doc, newchild); } @@ -374,17 +399,30 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) { + /* Spec link: https://dom.spec.whatwg.org/#dom-childnode-before */ + xmlNode *nextsib = dom_object_get_node(context); - xmlNodePtr newchild, prevsib, parentNode; - xmlNode *fragment, *afternextsib; + xmlNodePtr newchild, parentNode; + xmlNode *fragment; xmlDoc *doc; - bool beforefirstchild; - doc = nextsib->doc; - prevsib = nextsib->prev; - afternextsib = nextsib->next; + /* Spec step 1 */ parentNode = nextsib->parent; - beforefirstchild = !prevsib; + + /* Spec step 2 appears to be handled by dom_zvals_to_fragment */ + + /* Spec step 3: find first following child not in nodes; otherwise null */ + xmlNodePtr viable_previous_sibling = nextsib->prev; + while (viable_previous_sibling) { + if (!dom_is_node_in_list(nodes, nodesc, viable_previous_sibling)) { + break; + } + viable_previous_sibling = viable_previous_sibling->prev; + } + + doc = nextsib->doc; + + /* Spec step 4: convert nodes into fragment */ fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); if (fragment == NULL) { @@ -394,37 +432,14 @@ 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; + /* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */ + if (!viable_previous_sibling) { + viable_previous_sibling = parentNode->children; } 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; + viable_previous_sibling = viable_previous_sibling->next; } - - if (parentNode->children == nextsib) { - parentNode->children = newchild; - } else { - prevsib->next = newchild; - } - - fragment->last->next = nextsib; - if (nextsib) { - nextsib->prev = fragment->last; - } - - newchild->prev = prevsib; + /* Step 6: place fragment into the parent after viable_previous_sibling */ + dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment); dom_fragment_assign_parent_node(parentNode, fragment); dom_reconcile_ns(doc, newchild); diff --git a/ext/dom/tests/bug80602.phpt b/ext/dom/tests/bug80602.phpt index 9f041f686f516..844d829cb08d0 100644 --- a/ext/dom/tests/bug80602.phpt +++ b/ext/dom/tests/bug80602.phpt @@ -8,84 +8,84 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before($target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "1 ", $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; +echo "2 ", $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; +echo "3 ", $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; +echo "4 ", $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; +echo "5 ", $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; +echo "6 ", $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; +echo "7 ", $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; +echo "8 ", $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; +echo "9 ", $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; +echo "10 ", $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; +echo "11 ", $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; +echo "12 ", $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; +echo "13 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -93,19 +93,19 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before($target, 'bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "14 ", $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; +echo "15 ", $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; +echo "16 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -113,21 +113,21 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->before('bar', $target, $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "17 ", $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; +echo "18 ", $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; +echo "19 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -136,43 +136,43 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->before('bar', $doc->documentElement->firstChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "20 ", $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; +echo "21 ", $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; +echo "22 ", $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 +1 foo +2 foo +3 foo +4 foo +5 foo +6 foo +7 foo +8 foo +9 barbazfoo +10 foobarbaz +11 foobarbaz +12 barfoobaz +13 barbazfoo +14 foobarbaz +15 foobarbaz +16 foobarbaz +17 barfoo +18 foobar +19 foobar +20 barfoo +21 foobar +22 foobar diff --git a/ext/dom/tests/bug80602_2.phpt b/ext/dom/tests/bug80602_2.phpt index 1151417c0f845..7c5070f51424c 100644 --- a/ext/dom/tests/bug80602_2.phpt +++ b/ext/dom/tests/bug80602_2.phpt @@ -8,84 +8,84 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after($target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "1 ", $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; +echo "2 ", $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; +echo "3 ", $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; +echo "4 ", $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; +echo "5 ", $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; +echo "6 ", $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; +echo "7 ", $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; +echo "8 ", $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; +echo "9 ", $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; +echo "10 ", $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; +echo "11 ", $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; +echo "12 ", $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; +echo "13 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -93,19 +93,19 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after($target, 'bar','baz'); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "14 ", $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; +echo "15 ", $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; +echo "16 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -113,21 +113,21 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->firstChild; $target->after('bar', $target, $doc->documentElement->lastChild); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "17 ", $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; +echo "18 ", $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; +echo "19 ", $doc->saveXML($doc->documentElement).PHP_EOL; @@ -136,43 +136,43 @@ $doc = new \DOMDocument(); $doc->loadXML('foo'); $target = $doc->documentElement->lastChild; $target->after('bar', $doc->documentElement->firstChild, $target); -echo $doc->saveXML($doc->documentElement).PHP_EOL; +echo "20 ", $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; +echo "21 ", $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; +echo "22 ", $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 +1 foo +2 foo +3 foo +4 foo +5 foo +6 foo +7 foo +8 foo +9 foobarbaz +10 foobarbaz +11 foobarbaz +12 barfoobaz +13 barbazfoo +14 foobarbaz +15 foobarbaz +16 foobarbaz +17 barfoo +18 foobar +19 foobar +20 barfoo +21 foobar +22 foobar diff --git a/ext/dom/tests/bug80602_3.phpt b/ext/dom/tests/bug80602_3.phpt new file mode 100644 index 0000000000000..f9bf67e778da5 --- /dev/null +++ b/ext/dom/tests/bug80602_3.phpt @@ -0,0 +1,120 @@ +--TEST-- +Bug #80602 (Segfault when using DOMChildNode::before()) - use-after-free variation +--FILE-- +loadXML('foo'); +$target = $doc->documentElement->lastChild; +$target->before('bar', $doc->documentElement->firstChild, 'baz'); +echo $doc->saveXML($doc->documentElement), "\n"; +var_dump($target); + +$doc = new \DOMDocument(); +$doc->loadXML('foo'); +$target = $doc->documentElement->lastChild; +// Note: after instead of before +$target->after('bar', $doc->documentElement->firstChild, 'baz'); +echo $doc->saveXML($doc->documentElement), "\n"; +var_dump($target); + +?> +--EXPECTF-- +barfoobaz +object(DOMElement)#3 (23) { + ["schemaTypeInfo"]=> + NULL + ["tagName"]=> + string(4) "last" + ["firstElementChild"]=> + NULL + ["lastElementChild"]=> + NULL + ["childElementCount"]=> + int(0) + ["previousElementSibling"]=> + NULL + ["nextElementSibling"]=> + NULL + ["nodeName"]=> + string(4) "last" + ["nodeValue"]=> + string(0) "" + ["nodeType"]=> + int(1) + ["parentNode"]=> + string(22) "(object value omitted)" + ["childNodes"]=> + string(22) "(object value omitted)" + ["firstChild"]=> + NULL + ["lastChild"]=> + NULL + ["previousSibling"]=> + string(22) "(object value omitted)" + ["nextSibling"]=> + NULL + ["attributes"]=> + string(22) "(object value omitted)" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["namespaceURI"]=> + NULL + ["prefix"]=> + string(0) "" + ["localName"]=> + string(4) "last" + ["baseURI"]=> + string(%d) %s + ["textContent"]=> + string(0) "" +} +barfoobaz +object(DOMElement)#2 (23) { + ["schemaTypeInfo"]=> + NULL + ["tagName"]=> + string(4) "last" + ["firstElementChild"]=> + NULL + ["lastElementChild"]=> + NULL + ["childElementCount"]=> + int(0) + ["previousElementSibling"]=> + NULL + ["nextElementSibling"]=> + NULL + ["nodeName"]=> + string(4) "last" + ["nodeValue"]=> + string(0) "" + ["nodeType"]=> + int(1) + ["parentNode"]=> + string(22) "(object value omitted)" + ["childNodes"]=> + string(22) "(object value omitted)" + ["firstChild"]=> + NULL + ["lastChild"]=> + NULL + ["previousSibling"]=> + NULL + ["nextSibling"]=> + string(22) "(object value omitted)" + ["attributes"]=> + string(22) "(object value omitted)" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["namespaceURI"]=> + NULL + ["prefix"]=> + string(0) "" + ["localName"]=> + string(4) "last" + ["baseURI"]=> + string(%d) %s + ["textContent"]=> + string(0) "" +} diff --git a/ext/dom/tests/bug80602_4.phpt b/ext/dom/tests/bug80602_4.phpt new file mode 100644 index 0000000000000..a1df8d10caa31 --- /dev/null +++ b/ext/dom/tests/bug80602_4.phpt @@ -0,0 +1,33 @@ +--TEST-- +Bug #80602 (Segfault when using DOMChildNode::before()) - after text merge variation +--FILE-- +loadXML('foobar'); +$foo = $doc->firstChild->firstChild; +$bar = $doc->firstChild->lastChild; + +$foo->after($bar); + +var_dump($doc->saveXML()); + +$foo->nodeValue = "x"; + +var_dump($doc->saveXML()); + +$bar->nodeValue = "y"; + +var_dump($doc->saveXML()); + +?> +--EXPECT-- +string(43) " +foobar +" +string(41) " +xbar +" +string(39) " +xy +" diff --git a/ext/dom/tests/gh11288.phpt b/ext/dom/tests/gh11288.phpt new file mode 100644 index 0000000000000..fb8f7b541adb2 --- /dev/null +++ b/ext/dom/tests/gh11288.phpt @@ -0,0 +1,79 @@ +--TEST-- +GH-11288 (Error: Couldn't fetch DOMElement introduced in 8.2.6, 8.1.19) +--FILE-- + + +Loremipsum + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $fragment = $dom->createDocumentFragment(); + $fragment->append(...$span->childNodes); + $span->parentNode?->replaceChild($fragment, $span); + } +} + +var_dump($dom->saveHTML()); + +$html = << + +Loremipsum + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $span->replaceWith(...$span->childNodes); + } +} + +var_dump($dom->saveHTML()); + +$html = << + +Loremipsum + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $span->replaceWith('abc'); + } +} + +var_dump($dom->saveHTML()); +?> +--EXPECT-- +string(112) " + +Loremipsum + +" +string(112) " + +Loremipsum + +" +string(48) " + +abc + +" diff --git a/ext/dom/tests/gh11289.phpt b/ext/dom/tests/gh11289.phpt new file mode 100644 index 0000000000000..48cd2bb4a5a36 --- /dev/null +++ b/ext/dom/tests/gh11289.phpt @@ -0,0 +1,34 @@ +--TEST-- +GH-11289 (DOMException: Not Found Error introduced in 8.2.6, 8.1.19) +--FILE-- + + + +
+ + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$divs = iterator_to_array($dom->getElementsByTagName('div')->getIterator()); +foreach ($divs as $div) { + $fragment = $dom->createDocumentFragment(); + $fragment->appendXML('

Hi!

'); + $div->replaceWith(...$fragment->childNodes); +} + +var_dump($dom->saveHTML()); +?> +--EXPECT-- +string(61) " + + +

Hi!

+ + +" diff --git a/ext/dom/tests/gh11290.phpt b/ext/dom/tests/gh11290.phpt new file mode 100644 index 0000000000000..fa8a6cf0e2239 --- /dev/null +++ b/ext/dom/tests/gh11290.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-11290 (DOMElement::replaceWith causes crash) +--FILE-- + + + +

Loremipsumdolor

+ + +HTML; + +$dom = new DOMDocument(); +$dom->loadHTML($html); + +$spans = iterator_to_array($dom->getElementsByTagName('span')->getIterator()); +foreach ($spans as $span) { + if ('unwrap_me' === $span->getAttribute('class')) { + $span->replaceWith(...$span->childNodes); + } +} + +var_dump($dom->saveHTML()); +?> +--EXPECT-- +string(73) " + + +

Loremipsumdolor

+ + +" diff --git a/ext/dom/tests/gh9142.phpt b/ext/dom/tests/gh9142.phpt new file mode 100644 index 0000000000000..f72dfa823f38c --- /dev/null +++ b/ext/dom/tests/gh9142.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-9142 (DOMChildNode replaceWith() double-free error when replacing elements not separated by any whitespace) +--FILE-- +OneTwo'; + +($dom = new DOMDocument('1.0', 'UTF-8'))->loadHTML($document); + +foreach ((new DOMXPath($dom))->query('//var') as $var) { + $var->replaceWith($dom->createElement('p', $var->nodeValue)); +} + +var_dump($dom->saveHTML()); + +?> +--EXPECT-- +string(154) " +

One

Two

+" From 174a0f35dde9424c57b858a9873106faf56fa60f Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Wed, 24 May 2023 08:52:23 +0200 Subject: [PATCH 2/3] Fix Travis failure due to different formatting (?) --- ext/dom/tests/gh11288.phpt | 24 ++++++------------------ ext/dom/tests/gh11289.phpt | 10 ++-------- ext/dom/tests/gh11290.phpt | 10 ++-------- 3 files changed, 10 insertions(+), 34 deletions(-) diff --git a/ext/dom/tests/gh11288.phpt b/ext/dom/tests/gh11288.phpt index fb8f7b541adb2..f70bea80d9085 100644 --- a/ext/dom/tests/gh11288.phpt +++ b/ext/dom/tests/gh11288.phpt @@ -21,7 +21,7 @@ foreach ($spans as $span) { } } -var_dump($dom->saveHTML()); +var_dump(str_replace("\n", "", $dom->saveHTML())); $html = << @@ -40,7 +40,7 @@ foreach ($spans as $span) { } } -var_dump($dom->saveHTML()); +var_dump(str_replace("\n", "", $dom->saveHTML())); $html = << @@ -59,21 +59,9 @@ foreach ($spans as $span) { } } -var_dump($dom->saveHTML()); +var_dump(str_replace("\n", "", $dom->saveHTML())); ?> --EXPECT-- -string(112) " - -Loremipsum - -" -string(112) " - -Loremipsum - -" -string(48) " - -abc - -" +string(108) "Loremipsum" +string(108) "Loremipsum" +string(44) "abc" diff --git a/ext/dom/tests/gh11289.phpt b/ext/dom/tests/gh11289.phpt index 48cd2bb4a5a36..7771a486bd66b 100644 --- a/ext/dom/tests/gh11289.phpt +++ b/ext/dom/tests/gh11289.phpt @@ -22,13 +22,7 @@ foreach ($divs as $div) { $div->replaceWith(...$fragment->childNodes); } -var_dump($dom->saveHTML()); +var_dump(str_replace("\n", "", $dom->saveHTML())); ?> --EXPECT-- -string(61) " - - -

Hi!

- - -" +string(55) "

Hi!

" diff --git a/ext/dom/tests/gh11290.phpt b/ext/dom/tests/gh11290.phpt index fa8a6cf0e2239..2900720301041 100644 --- a/ext/dom/tests/gh11290.phpt +++ b/ext/dom/tests/gh11290.phpt @@ -21,13 +21,7 @@ foreach ($spans as $span) { } } -var_dump($dom->saveHTML()); +var_dump(str_replace("\n", "", $dom->saveHTML())); ?> --EXPECT-- -string(73) " - - -

Loremipsumdolor

- - -" +string(67) "

Loremipsumdolor

" From 53e3feb4770bf3e8456d489501085071f089418d Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Thu, 25 May 2023 21:58:31 +0200 Subject: [PATCH 3/3] Fixup spec step 2 --- ext/dom/parentnode.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index c132d6ddc3414..46c90a13e31d5 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -356,13 +356,12 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc) xmlNode *fragment; xmlDoc *doc; - int stricterror = dom_get_strict_error(context->document); - /* Spec step 1 */ parentNode = prevsib->parent; /* Spec step 2 */ if (!parentNode) { - php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror); + int stricterror = dom_get_strict_error(context->document); + php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); return; } @@ -408,8 +407,12 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc) /* Spec step 1 */ parentNode = nextsib->parent; - - /* Spec step 2 appears to be handled by dom_zvals_to_fragment */ + /* Spec step 2 */ + if (!parentNode) { + int stricterror = dom_get_strict_error(context->document); + php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + return; + } /* Spec step 3: find first following child not in nodes; otherwise null */ xmlNodePtr viable_previous_sibling = nextsib->prev;