Html Reader Converting Cell Containing 0 to Null String (#2813)

oleibman · web-flow · commit 070bc68514a1 · 2022-05-10T07:33:45.000-07:00
Fix #2810. Repairing some Phpstan diagnostics, used `?:` rather than `??` in a few places. 2 different Html modules are affected. Also, Ods Reader, but its problem is with sheet title rather than cell contents. And, as it turns out, Ods Reader was already not handling sheets with a title of `0` correctly - it made a truthy test before setting sheet title. That is now changed to truthy or numeric. Other readers are not susceptible to this problem. Tests are added.
diff --git a/src/PhpSpreadsheet/Calculation/ArrayEnabled.php b/src/PhpSpreadsheet/Calculation/ArrayEnabled.php
@@ -20,7 +20,7 @@ private static function initialiseHelper($arguments): void
         if (self::$arrayArgumentHelper === null) {
             self::$arrayArgumentHelper = new ArrayArgumentHelper();
         }
-        self::$arrayArgumentHelper->initialise($arguments ?: []);
+        self::$arrayArgumentHelper->initialise(($arguments === false) ? [] : $arguments);
     }
 
     /**
diff --git a/src/PhpSpreadsheet/Helper/Html.php b/src/PhpSpreadsheet/Helper/Html.php
@@ -795,7 +795,7 @@ protected function parseTextNode(DOMText $textNode): void
         $domText = preg_replace(
             '/\s+/u',
             ' ',
-            str_replace(["\r", "\n"], ' ', $textNode->nodeValue ?: '')
+            str_replace(["\r", "\n"], ' ', $textNode->nodeValue ?? '')
         );
         $this->stringData .= $domText;
         $this->buildTextRun();
diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php
@@ -619,7 +619,7 @@ protected function processDomElement(DOMNode $element, Worksheet $sheet, int &$r
     {
         foreach ($element->childNodes as $child) {
             if ($child instanceof DOMText) {
-                $domText = preg_replace('/\s+/u', ' ', trim($child->nodeValue ?: ''));
+                $domText = preg_replace('/\s+/u', ' ', trim($child->nodeValue ?? ''));
                 if (is_string($cellContent)) {
                     //    simply append the text if the cell content is a plain text string
                     $cellContent .= $domText;
diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php
@@ -325,7 +325,7 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)
                 }
                 $spreadsheet->setActiveSheetIndex($worksheetID);
 
-                if ($worksheetName) {
+                if ($worksheetName || is_numeric($worksheetName)) {
                     // Use false for $updateFormulaCellReferences to prevent adjustment of worksheet references in
                     // formula cells... during the load, all formulae should be correct, and we're simply
                     // bringing the worksheet name in line with the formula, not the reverse
@@ -628,7 +628,7 @@ private function lookForActiveSheet(DOMElement $settings, Spreadsheet $spreadshe
         foreach ($settings->getElementsByTagNameNS($configNs, 'config-item') as $t) {
             if ($t->getAttributeNs($configNs, 'name') === 'ActiveTable') {
                 try {
-                    $spreadsheet->setActiveSheetIndexByName($t->nodeValue ?: '');
+                    $spreadsheet->setActiveSheetIndexByName($t->nodeValue ?? '');
                 } catch (Throwable $e) {
                     // do nothing
                 }
diff --git a/tests/PhpSpreadsheetTests/Helper/HtmlTest.php b/tests/PhpSpreadsheetTests/Helper/HtmlTest.php
@@ -28,6 +28,7 @@ public function providerUtf8EncodingSupport(): array
             ['können', 'können'],
             ['русский', 'русский'],
             ["foo\nbar", '<p>foo</p><p>bar</p>'],
+            'issue2810' => ['0', '0'],
         ];
     }
 }
diff --git a/tests/PhpSpreadsheetTests/Reader/Html/Issue2810Test.php b/tests/PhpSpreadsheetTests/Reader/Html/Issue2810Test.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Html;
+
+use PhpOffice\PhpSpreadsheet\Reader\Html;
+use PHPUnit\Framework\TestCase;
+
+class Issue2810Test extends TestCase
+{
+    // Reader has been converting falsey values to null
+    public function testIssue2810(): void
+    {
+        $content = <<<'EOF'
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset='utf-8'>
+    <title>Declaracion en Linea</title>
+</head>
+<body>
+  <table>
+    <tr>
+      <td>1</td>
+      <td>0</td>
+      <td>2</td>
+    </tr>
+  </table>
+</body>
+</html>
+
+EOF;
+        $reader = new Html();
+        $spreadsheet = $reader->loadFromString($content);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame(1, $sheet->getCell('A1')->getValue());
+        self::assertSame(0, $sheet->getCell('B1')->getValue());
+        self::assertSame(2, $sheet->getCell('C1')->getValue());
+        $spreadsheet->disconnectWorksheets();
+    }
+}
diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/Issue2810Test.php b/tests/PhpSpreadsheetTests/Reader/Ods/Issue2810Test.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;
+
+use PhpOffice\PhpSpreadsheet\Reader\Ods;
+use PHPUnit\Framework\TestCase;
+
+class Issue2810Test extends TestCase
+{
+    public function testIssue2810(): void
+    {
+        // Active sheet with title of '0' wasn't found
+        $filename = 'tests/data/Reader/Ods/issue.2810.ods';
+        $reader = new Ods();
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame('Active', $sheet->getCell('A1')->getValue());
+        $spreadsheet->disconnectWorksheets();
+    }
+}
diff --git a/tests/data/Reader/Ods/issue.2810.ods b/tests/data/Reader/Ods/issue.2810.ods

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ private static function initialiseHelper($arguments): void`
`20`	`20`	`if (self::$arrayArgumentHelper === null) {`
`21`	`21`	`self::$arrayArgumentHelper = new ArrayArgumentHelper();`
`22`	`22`	`}`
`23`		`- self::$arrayArgumentHelper->initialise($arguments ?: []);`
	`23`	`+ self::$arrayArgumentHelper->initialise(($arguments === false) ? [] : $arguments);`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -619,7 +619,7 @@ protected function processDomElement(DOMNode $element, Worksheet $sheet, int &$r`
`619`	`619`	`{`
`620`	`620`	`foreach ($element->childNodes as $child) {`
`621`	`621`	`if ($child instanceof DOMText) {`
`622`		`- $domText = preg_replace('/\s+/u', ' ', trim($child->nodeValue ?: ''));`
	`622`	`+ $domText = preg_replace('/\s+/u', ' ', trim($child->nodeValue ?? ''));`
`623`	`623`	`if (is_string($cellContent)) {`
`624`	`624`	`// simply append the text if the cell content is a plain text string`
`625`	`625`	`$cellContent .= $domText;`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ public function providerUtf8EncodingSupport(): array`
`28`	`28`	`['können', 'können'],`
`29`	`29`	`['русский', 'русский'],`
`30`	`30`	`["foo\nbar", '<p>foo</p><p>bar</p>'],`
	`31`	`+ 'issue2810' => ['0', '0'],`
`31`	`32`	`];`
`32`	`33`	`}`
`33`	`34`	`}`