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

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.
PHPOffice · May 10, 2022 · 070bc68 · 070bc68
1 parent b0bfdde
commit 070bc68
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 5 deletions.
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