paquettg/php-html-parser

Numbers in comments cause fatal errors

leonk opened this issue · 1 comments

leonk commented

PHPHtmlParser will error when you try to output markup that has a comment, with a word that is just a number inside of it. Since it treats words inside comments as HTML attributes, and it assumes all attributes are strings.

<?php

require '../vendor/autoload.php';

use PHPHtmlParser\Dom;

$html = '
      <!-- Test comment with a number 1 -->
';

$dom = new Dom;

$dom->loadStr($html, ['cleanupInput' => FALSE]);

echo $dom->outerHtml;

Expected result:
<!-- Test comment with a number 1 -->

Actual result (error):

PHP Fatal error:  Uncaught TypeError: Argument 1 passed to PHPHtmlParser\Dom\Tag::getAttribute() must be of the type string, int given, called in /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/Tag.php on line 339 and defined in /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/Tag.php:301
Stack trace:
#0 /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/Tag.php(339): PHPHtmlParser\Dom\Tag->getAttribute(1)
#1 /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/HtmlNode.php(133): PHPHtmlParser\Dom\Tag->makeOpeningTag()
#2 /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/HtmlNode.php(94): PHPHtmlParser\Dom\HtmlNode->outerHtml()
#3 /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/HtmlNode.php(125): PHPHtmlParser\Dom\HtmlNode->innerHtml()
#4 /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/AbstractNode.php(99): PHPHtmlParser\Dom\HtmlNode->outerHtml()
#5 /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom.php(135): PHPHtmlParser in /vagrant/vendor/paquettg/php-html-parser/src/PHPHtmlParser/Dom/Tag.php on line 301

Note the above is using version 2.2.1, but from what I can see it's also an issue on 3.1.0

Also note that you would still get the error if you had an attribute that was just a number on a normal html element, and with cleanupInput enabled. However, as this is invalid HTML, I guess it's not an issue.

I merged in your PR for version 3.1 but I have no intention at the moment to support version 2.X of this package with fixes so I closed off that PR and removed the dev branch to remove that confusion.

Thank you for the contribution, it is very much appriciated.