ZipArchive and "Inconsistent" Zip File (#2376)

* ZipArchive and "Inconsistent" Zip File

Fix #2362. I added test for zip file inconsistency when dealing with a particularly nasty PHP/libzip bug affecting zero-length files. However, we also now verify that the file starts with a valid zip signature, so the consistency test is not really needed, and, from what I've read on the web, isn't particularly useful. The file with a problem, for example, opens just fine with Excel and zip, despite Php reporting it as inconsistent (when asked to check consistency). So, remove the consistency check.

* Update Issue2362Test.php

Latest Phpstan does not allow cast from 'mixed' to 'string'.

* Update Issue2362Test.php

src/PhpSpreadsheet/Reader/Ods.php

@@ -47,7 +47,7 @@ class Ods extends BaseReader
 
         // Load file
 
-        if (File::testFileNoThrow($filename)) {
+        if (File::testFileNoThrow($filename, '')) {
             $zip = new ZipArchive();
             if ($zip->open($filename) === true) {
                 // check if it is an OOXML archive

src/PhpSpreadsheet/Shared/File.php

@@ -61,7 +61,7 @@ class File
 
             if (self::validateZipFirst4($zipFile)) {
                 $zip = new ZipArchive();
-                $res = $zip->open($zipFile, ZipArchive::CHECKCONS);
+                $res = $zip->open($zipFile);
                 if ($res === true) {
                     $returnValue = ($zip->getFromName($archiveFile) !== false);
                     $zip->close();
@@ -164,23 +164,22 @@ class File
     /**
      * Same as assertFile, except return true/false and don't throw Exception.
      */
-    public static function testFileNoThrow(string $filename, string $zipMember = ''): bool
+    public static function testFileNoThrow(string $filename, ?string $zipMember = null): bool
     {
         if (!is_file($filename)) {
             return false;
         }
-
         if (!is_readable($filename)) {
             return false;
         }
-
+        if ($zipMember === null) {
-        if ($zipMember !== '') {
-            $zipfile = "zip://$filename#$zipMember";
-            if (!self::fileExists($zipfile)) {
-                return false;
-            }
-        }
-
             return true;
         }
+        // validate zip, but don't check specific member
+        if ($zipMember === '') {
+            return self::validateZipFirst4($filename);
+        }
+
+        return self::fileExists("zip://$filename#$zipMember");
+    }
 }

tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2362Test.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\RichText\RichText;
+use PHPUnit\Framework\TestCase;
+use ZipArchive;
+
+class Issue2362Test extends TestCase
+{
+    public function testPreliminaries(): void
+    {
+        // ZipArchive says file is 'inconsistent',
+        // but Excel has no problem with it.
+        $filename = 'tests/data/Reader/XLSX/issue.2362.xlsx';
+        $zip = new ZipArchive();
+        $res = $zip->open($filename, ZipArchive::CHECKCONS);
+        self::assertSame(ZipArchive::ER_INCONS, $res);
+    }
+
+    public function testIssue2362(): void
+    {
+        $filename = 'tests/data/Reader/XLSX/issue.2362.xlsx';
+        $reader = IOFactory::createReader('Xlsx');
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getActiveSheet();
+
+        $value = $sheet->getCell('A1')->getValue();
+        if ($value instanceof RichText) {
+            self::assertSame('Дата', (string) $value);
+        } else {
+            self::fail('A1 is not RichText');
+        }
+        $value = $sheet->getCell('D21')->getValue();
+        if ($value instanceof RichText) {
+            self::assertSame('391800, Рязанская область, г. Скопин, ул. Ленина, д. 40', (string) $value);
+        } else {
+            self::fail('D21 is not RichText');
+        }
+        $spreadsheet->disconnectWorksheets();
+    }
+}