Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOMElement::replaceWith causes crash #11290

Closed
fluffycondor opened this issue May 22, 2023 · 2 comments
Closed

DOMElement::replaceWith causes crash #11290

fluffycondor opened this issue May 22, 2023 · 2 comments

Comments

@fluffycondor
Copy link

Description

This is a longstanding one (sorry for the php-dom day!). Sometimes the same code from #11288 causes crash on every 8.x PHP version. The code is the same, but the html markup is slightly differs.

Live reproducing:
https://3v4l.org/KjJgQ // On every PHP 8.x version throws DOMException: Not Found Error, hangs for 1-15 seconds, then crashes.
https://3v4l.org/FdOZR // Hangs for 1-15 seconds, crashes on every PHP versions but PHP 8.2.6 and 8.1.19.

It's kinda related to #11289 and #11288, but this is a longstanding one. I guess this is a part of a big picture, so I hope this would help to investigate the main cause of weird php-dom behavior.

The following code:

<?php
$html = <<<HTML
<!DOCTYPE HTML>
<html>
<body>
    <p><span class="unwrap_me">Lorem</span><span class="unwrap_me">ipsum</span><span class="unwrap_me">dolor</span></p>
</body>
</html>
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);
    }
}

results in crash on every PHP 8.x version.

PHP Version

PHP 8.x

Operating System

No response

@fluffycondor
Copy link
Author

Oh, looks like this could be a duplicate of #9142.

@nielsdos
Copy link
Member

nielsdos commented May 22, 2023

This one's also fixed by #11299, so I'll amend that PR to include this test. Thanks for the report & testcase!

nielsdos added a commit to nielsdos/php-src that referenced this issue May 22, 2023
…placeWith

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.
nielsdos added a commit to nielsdos/php-src that referenced this issue May 22, 2023
…ception 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.
nielsdos added a commit that referenced this issue May 25, 2023
… segfaults 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.

Closes GH-11299.
nielsdos added a commit that referenced this issue May 25, 2023
* PHP-8.1:
  Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMExceptions and segfaults with replaceWith
nielsdos added a commit that referenced this issue May 25, 2023
* PHP-8.2:
  Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMExceptions and segfaults with replaceWith
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants