Skip to content

Commit

Permalink
Prevent DocumentFragment from causing internal PHP error by using app…
Browse files Browse the repository at this point in the history
…end() (#454)

* fix: DocumentFragment append does not cause error
closes #453

* ci: try dev master branch

* ci: try dev master branch

* test: satisfy phpstan

* ci: upgrade phpunit to v4
  • Loading branch information
g105b authored Apr 25, 2024
1 parent b3fe4a5 commit e2460c3
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
run: tar -xvf /tmp/github-actions/build.tar ./

- name: PHP Unit tests
uses: php-actions/phpunit@v3
uses: php-actions/phpunit@v4
env:
XDEBUG_MODE: cover
with:
Expand Down
12 changes: 9 additions & 3 deletions src/ParentNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,19 @@ protected function __prop_get_children():HTMLCollection {
* returns the appended Node object.
* + Element.append() can append several nodes and strings, whereas
* Node.appendChild() can only append one node.
* @param Node|Element|Text|Comment|string...$nodes
* @param Node|Element|Text|Comment|DocumentFragment|string...$nodes
*/
public function append(...$nodes):void {
// Without this clumsy iteration, PHP 8.1 throws "free(): double free detected in tcache 2"
foreach($nodes as $node) {
/** @phpstan-ignore-next-line libxml's DOMNode does not define append() */
parent::append($node);
// And without this clumsy if/else, PHP 8.3 throws "double free or corruption (!prev)"
if(is_string($node)) {
/** @phpstan-ignore-next-line libxml's DOMNode does not define append() */
parent::append($node);
}
else {
parent::appendChild($node);
}
}
}

Expand Down
14 changes: 14 additions & 0 deletions test/phpunit/DocumentFragmentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,18 @@ public function testGetElementById():void {
$sut->appendChild($nodeWithId);
self::assertSame($nodeWithId, $sut->getElementById("test"));
}

public function testAppendMultipleNodesThenAddToParentElement():void {
$document = new HTMLDocument();
$sut = $document->createDocumentFragment();
$expectedString = "";
for($i = 0; $i < 10; $i++) {
$textNode = $document->createTextNode("Node$i");
$sut->append($textNode);
$expectedString .= "Node$i";
}

$document->documentElement->append($sut);
self::assertSame($expectedString, $document->textContent);
}
}

0 comments on commit e2460c3

Please sign in to comment.