Skip to content

Commit 35e1009

Browse files
oleibmanoleibman
authored
Merge pull request #5054 from oleibman/libxmlbreak2
Breaking Changes in libxml Affecting Helper\Html and Reader\Html
2 parents 6720b53 + f0e66c2 commit 35e1009

5 files changed

Lines changed: 57 additions & 21 deletions

File tree

‎src/PhpSpreadsheet/Helper/Html.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
use DOMElement;
88
use DOMNode;
99
use DOMText;
10+
use PhpOffice\PhpSpreadsheet\Reader\Csv;
11+
use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
1012
use PhpOffice\PhpSpreadsheet\RichText\RichText;
1113
use PhpOffice\PhpSpreadsheet\Style\Color;
1214
use PhpOffice\PhpSpreadsheet\Style\Font;
@@ -581,6 +583,8 @@ public function __construct()
581583
's' => $this->startStrikethruTag(...),
582584
'sup' => $this->startSuperscriptTag(...),
583585
'sub' => $this->startSubscriptTag(...),
586+
'ul' => $this->ulOlTag(...),
587+
'ol' => $this->ulOlTag(...),
584588
];
585589
}
586590
if (!isset($this->endTagCallbacks)) {
@@ -624,13 +628,18 @@ private function initialise(): void
624628
public function toRichTextObject(string $html, bool $preserveWhiteSpace = false): RichText
625629
{
626630
$this->initialise();
631+
$scanner = new XmlScanner(XmlScanner::HTML_PATTERN);
632+
$html = $scanner->scan($html);
627633

628634
// Create a new DOM object
629635
$dom = new DOMDocument();
636+
637+
// Start with UTF-8 BOM so that loadHTML below will use correct charset
638+
$loadString = Csv::UTF8_BOM . $html;
639+
630640
// Load the HTML file into the DOM object
631641
// Note the use of error suppression, because typically this will be an html fragment, so not fully valid markup
632-
$prefix = '<?xml encoding="UTF-8">';
633-
@$dom->loadHTML($prefix . $html, LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD);
642+
@$dom->loadHTML($loadString, LIBXML_HTML_NODEFDTD);
634643
// Discard excess white space
635644
$dom->preserveWhiteSpace = false;
636645

@@ -806,6 +815,13 @@ protected function endStrikethruTag(): void
806815
$this->strikethrough = false;
807816
}
808817

818+
protected function ulOlTag(): void
819+
{
820+
if (substr($this->stringData, -1) !== "\n") {
821+
$this->breakTag();
822+
}
823+
}
824+
809825
public function breakTag(): void
810826
{
811827
$this->stringData .= "\n";

‎src/PhpSpreadsheet/Reader/Security/XmlScanner.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,17 @@ class XmlScanner
1414
/** @var ?callable */
1515
private $callback;
1616

17-
public function __construct(string $pattern = '<!DOCTYPE')
17+
public const HTML_PATTERN = '<!ENTITY';
18+
public const XML_PATTERN = '<!DOCTYPE';
19+
20+
public function __construct(string $pattern = self::XML_PATTERN)
1821
{
1922
$this->pattern = $pattern;
2023
}
2124

2225
public static function getInstance(Reader\IReader $reader): self
2326
{
24-
$pattern = ($reader instanceof Reader\Html) ? '<!ENTITY' : '<!DOCTYPE';
27+
$pattern = ($reader instanceof Reader\Html) ? self::HTML_PATTERN : self::XML_PATTERN;
2528

2629
return new self($pattern);
2730
}

‎tests/PhpSpreadsheetTests/Reader/Html/HtmlLibxmlTest.php‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
namespace PhpOffice\PhpSpreadsheetTests\Reader\Html;
66

77
use PhpOffice\PhpSpreadsheet\Reader\Html;
8-
use PhpOffice\PhpSpreadsheet\Spreadsheet;
98
use PHPUnit\Framework\TestCase;
109

1110
/**
@@ -16,65 +15,74 @@
1615
*/
1716
class HtmlLibxmlTest extends TestCase
1817
{
19-
private Spreadsheet $spreadsheet;
20-
21-
protected function tearDown(): void
18+
public function testAll(): void
2219
{
23-
$this->spreadsheet->disconnectWorksheets();
24-
unset($this->spreadsheet);
20+
if (LIBXML_VERSION >= 21400) {
21+
self::markTestSkipped('Current version of libxml has invalidated these tests');
22+
}
23+
$this->xtestLoadInvalidString();
24+
$this->xtestLoadValidString();
25+
$this->xtestLoadValidStringNoSuppress();
26+
$this->xtestLoadValidFile();
27+
$this->xtestLoadInvalidFile();
2528
}
2629

27-
public function testLoadInvalidString(): void
30+
public function xtestLoadInvalidString(): void
2831
{
2932
$libxml = libxml_use_internal_errors();
3033
$html = '<table<>';
3134
$reader = new Html();
3235
$reader->setSuppressLoadWarnings(true);
33-
$this->spreadsheet = $reader->loadFromString($html);
36+
$spreadsheet = $reader->loadFromString($html);
3437
self::assertNotEmpty($reader->getLibxmlMessages());
3538
self::assertSame($libxml, libxml_use_internal_errors());
39+
$spreadsheet->disconnectWorksheets();
3640
}
3741

38-
public function testLoadValidString(): void
42+
public function xtestLoadValidString(): void
3943
{
4044
$libxml = libxml_use_internal_errors();
4145
$html = '<table>';
4246
$reader = new Html();
4347
$reader->setSuppressLoadWarnings(true);
44-
$this->spreadsheet = $reader->loadFromString($html);
48+
$spreadsheet = $reader->loadFromString($html);
4549
self::assertEmpty($reader->getLibxmlMessages());
4650
self::assertSame($libxml, libxml_use_internal_errors());
51+
$spreadsheet->disconnectWorksheets();
4752
}
4853

49-
public function testLoadValidStringNoSuppress(): void
54+
public function xtestLoadValidStringNoSuppress(): void
5055
{
5156
$libxml = libxml_use_internal_errors();
5257
$html = '<table>';
5358
$reader = new Html();
54-
$this->spreadsheet = $reader->loadFromString($html);
59+
$spreadsheet = $reader->loadFromString($html);
5560
self::assertEmpty($reader->getLibxmlMessages());
5661
self::assertSame($libxml, libxml_use_internal_errors());
62+
$spreadsheet->disconnectWorksheets();
5763
}
5864

59-
public function testLoadValidFile(): void
65+
public function xtestLoadValidFile(): void
6066
{
6167
$libxml = libxml_use_internal_errors();
6268
$reader = new Html();
6369
$reader->setSuppressLoadWarnings(true);
6470
$file = 'tests/data/Reader/HTML/charset.ISO-8859-1.html4.html';
65-
$this->spreadsheet = $reader->load($file);
71+
$spreadsheet = $reader->load($file);
6672
self::assertEmpty($reader->getLibxmlMessages());
6773
self::assertSame($libxml, libxml_use_internal_errors());
74+
$spreadsheet->disconnectWorksheets();
6875
}
6976

70-
public function testLoadInvalidFile(): void
77+
public function xtestLoadInvalidFile(): void
7178
{
7279
$libxml = libxml_use_internal_errors();
7380
$reader = new Html();
7481
$reader->setSuppressLoadWarnings(true);
7582
$file = 'tests/data/Reader/HTML/badhtml.html';
76-
$this->spreadsheet = $reader->load($file);
83+
$spreadsheet = $reader->load($file);
7784
self::assertNotEmpty($reader->getLibxmlMessages());
7885
self::assertSame($libxml, libxml_use_internal_errors());
86+
$spreadsheet->disconnectWorksheets();
7987
}
8088
}

‎tests/PhpSpreadsheetTests/Reader/Html/HtmlPhpunit10Test.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,16 @@ public static function errorHandler(int $errno, string $errstr): bool
3737
return false; // continue error handling
3838
}
3939

40+
private function checkLibxmlVersion(): void
41+
{
42+
if (LIBXML_VERSION >= 21400) {
43+
self::markTestSkipped('Current version of libxml has invalidated this test');
44+
}
45+
}
46+
4047
public function testBadHtml(): void
4148
{
49+
$this->checkLibxmlVersion();
4250
$filename = 'tests/data/Reader/HTML/badhtml.html';
4351
$reader = new Html();
4452
self::assertTrue($reader->canRead($filename));
@@ -48,6 +56,7 @@ public function testBadHtml(): void
4856

4957
public function testLoadInvalidString(): void
5058
{
59+
$this->checkLibxmlVersion();
5160
$html = '<table<>';
5261
(new Html())->loadFromString($html);
5362
self::assertStringContainsString('DOMDocument::loadHTML', self::$errorString);

‎tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public function testComments(RichText $richText, string $expected): void
9999
$spreadsheet->disconnectWorksheets();
100100

101101
$actual = $reloadedSpreadsheet->getActiveSheet()->getComment('A1')->getText()->getPlainText();
102-
self::assertSame($richText->getPlainText(), $actual);
102+
self::assertStringEqualsStringIgnoringLineEndings($richText->getPlainText(), $actual);
103103
$reloadedSpreadsheet->disconnectWorksheets();
104104
}
105105
}

0 commit comments

Comments
 (0)