From 04e7c3075803f6d6e6bbf1ce560badae356e6225 Mon Sep 17 00:00:00 2001 From: oleibman Date: Wed, 3 Mar 2021 01:52:11 -0800 Subject: [PATCH] Fix Two 32-bit Timestamp Problems, and Minor getFormattedValue Bug (#1891) I ran the test suite using 32-bit PHP. There were 2 places where changes were needed due to 32-bit timestamps. Reader\\Xml.php was using strtotime as an intermediate step in converting a string timestamp to an Excel timestamp. The XML file type stores pure timestamps (i.e. no date portion) as, e.g., 1899-12-31T02:30:00.000, and that value causes an error using strtotime on a 32-bit system. However, it is sufficient to use that value in a DateTime constructor, and that will work for 32- and 64-bit. There was no test for that particular cell, so I added one to the XML read test. And that's when I discovered the getFormattedValue bug. The cell's format is `hh":"mm":"ss`. The quotes around the colons are disrupting the formatting. PhpSpreadsheet formats the cell by converting the Excel format to a Php Date format, in this case `H\:m\:s`. That's a problem, since Excel thinks 'm' means *minutes*, but PHP thinks it means *months*. This is not a problem when the colon is not quoted; there are ample tests for that. I added my best guess as to how to recognize this situation, changing `\:m` to `:i`. The XML read test now succeeds, and no other tests were broken by this change. Test Shared\\DateTest had one test where the expected result of converting to a Unix timestamp exceeds 2**32. Since a Unix timestamp is strictly an int, that test fails on a 32-bit system. In the discussion regarding recently merged PR #1870, it was felt that the user base might still be using the functions that convert to and from a timestamp. So, we should not drop this test, but, since it cannot succeed on a 32-bit system, I changed it to be skipped whenever the expected result exceeded PHP_INT_MAX. There are 3 "toTimestamp" functions within that test. Only one of these had been affected, but I thought it was a good idea to add additional tests to the others to demonstrate this condition. In the course of testing, I also discovered some 32-bit problems with bitwise and base-conversion functions. I am preparing separate PRs to deal with those. --- src/PhpSpreadsheet/Reader/Xml.php | 5 ++++- src/PhpSpreadsheet/Style/NumberFormat.php | 4 ++++ .../PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php | 4 ++++ tests/PhpSpreadsheetTests/Shared/DateTest.php | 13 +++++++++++++ tests/data/Shared/Date/ExcelToTimestamp1900.php | 7 +++++++ tests/data/Shared/Date/ExcelToTimestamp1904.php | 5 +++++ 6 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Reader/Xml.php b/src/PhpSpreadsheet/Reader/Xml.php index f38a951562..b1df0ef5e9 100644 --- a/src/PhpSpreadsheet/Reader/Xml.php +++ b/src/PhpSpreadsheet/Reader/Xml.php @@ -2,6 +2,8 @@ namespace PhpOffice\PhpSpreadsheet\Reader; +use DateTime; +use DateTimeZone; use PhpOffice\PhpSpreadsheet\Cell\AddressHelper; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Cell\DataType; @@ -557,7 +559,8 @@ public function loadIntoExisting($pFilename, Spreadsheet $spreadsheet) break; case 'DateTime': $type = DataType::TYPE_NUMERIC; - $cellValue = Date::PHPToExcel(strtotime($cellValue . ' UTC')); + $dateTime = new DateTime($cellValue, new DateTimeZone('UTC')); + $cellValue = Date::PHPToExcel($dateTime); break; case 'Error': diff --git a/src/PhpSpreadsheet/Style/NumberFormat.php b/src/PhpSpreadsheet/Style/NumberFormat.php index a96d2ac3cb..a623657b2c 100644 --- a/src/PhpSpreadsheet/Style/NumberFormat.php +++ b/src/PhpSpreadsheet/Style/NumberFormat.php @@ -502,6 +502,10 @@ private static function formatAsDate(&$value, &$format): void $format = preg_replace_callback('/"(.*)"/U', ['self', 'escapeQuotesCallback'], $format); $dateObj = Date::excelToDateTimeObject($value); + // If the colon preceding minute had been quoted, as happens in + // Excel 2003 XML formats, m will not have been changed to i above. + // Change it now. + $format = \preg_replace('/\\\\:m/', ':i', $format); $value = $dateObj->format($format); } diff --git a/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php b/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php index 969571f2a5..3783c1a948 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php @@ -36,6 +36,10 @@ public function testLoad(): void self::assertEquals('# ?0/??0', $sheet->getCell('A11')->getStyle()->getNumberFormat()->getFormatCode()); // Same pattern, same value, different display in Gnumeric vs Excel //self::assertEquals('1 1/2', $sheet->getCell('A11')->getFormattedValue()); + self::assertEquals('hh":"mm":"ss', $sheet->getCell('A13')->getStyle()->getNumberFormat()->getFormatCode()); + self::assertEquals('02:30:00', $sheet->getCell('A13')->getFormattedValue()); + self::assertEquals('d/m/yy hh":"mm', $sheet->getCell('A15')->getStyle()->getNumberFormat()->getFormatCode()); + self::assertEquals('19/12/60 01:30', $sheet->getCell('A15')->getFormattedValue()); self::assertEquals('=B1+C1', $sheet->getCell('H1')->getValue()); self::assertEquals('=E2&F2', $sheet->getCell('J2')->getValue()); diff --git a/tests/PhpSpreadsheetTests/Shared/DateTest.php b/tests/PhpSpreadsheetTests/Shared/DateTest.php index 7254635ea3..550e2f6a9c 100644 --- a/tests/PhpSpreadsheetTests/Shared/DateTest.php +++ b/tests/PhpSpreadsheetTests/Shared/DateTest.php @@ -8,16 +8,20 @@ class DateTest extends TestCase { + private $excelCalendar; + private $dttimezone; protected function setUp(): void { $this->dttimezone = Date::getDefaultTimeZone(); + $this->excelCalendar = Date::getExcelCalendar(); } protected function tearDown(): void { Date::setDefaultTimeZone($this->dttimezone); + Date::setExcelCalendar($this->excelCalendar); } public function testSetExcelCalendar(): void @@ -47,6 +51,9 @@ public function testSetExcelCalendarWithInvalidValue(): void */ public function testDateTimeExcelToTimestamp1900($expectedResult, ...$args): void { + if (is_numeric($expectedResult) && ($expectedResult > PHP_INT_MAX || $expectedResult < PHP_INT_MIN)) { + self::markTestSkipped('Test invalid on 32-bit system.'); + } Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900); $result = Date::excelToTimestamp(...$args); @@ -119,6 +126,9 @@ public function providerDateTimeFormattedPHPToExcel1900() */ public function testDateTimeExcelToTimestamp1904($expectedResult, ...$args): void { + if (is_numeric($expectedResult) && ($expectedResult > PHP_INT_MAX || $expectedResult < PHP_INT_MIN)) { + self::markTestSkipped('Test invalid on 32-bit system.'); + } Date::setExcelCalendar(Date::CALENDAR_MAC_1904); $result = Date::excelToTimestamp(...$args); @@ -171,6 +181,9 @@ public function providerIsDateTimeFormatCode() */ public function testDateTimeExcelToTimestamp1900Timezone($expectedResult, ...$args): void { + if (is_numeric($expectedResult) && ($expectedResult > PHP_INT_MAX || $expectedResult < PHP_INT_MIN)) { + self::markTestSkipped('Test invalid on 32-bit system.'); + } Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900); $result = Date::excelToTimestamp(...$args); diff --git a/tests/data/Shared/Date/ExcelToTimestamp1900.php b/tests/data/Shared/Date/ExcelToTimestamp1900.php index ab79731e3a..ffcd194f46 100644 --- a/tests/data/Shared/Date/ExcelToTimestamp1900.php +++ b/tests/data/Shared/Date/ExcelToTimestamp1900.php @@ -72,4 +72,11 @@ 10666, 0.12345, ], + // 29-Apr-2038 00:00:00 beyond PHP 32-bit Latest Date + [ + 2156112000, + 50524, + ], + [-2147483648, -2147483648 / 86400], // Okay on 64- and 32-bit systems + [-2147483649, -2147483649 / 86400], // Skipped test on 32-bit ]; diff --git a/tests/data/Shared/Date/ExcelToTimestamp1904.php b/tests/data/Shared/Date/ExcelToTimestamp1904.php index 1c013ab4ef..fc55238af7 100644 --- a/tests/data/Shared/Date/ExcelToTimestamp1904.php +++ b/tests/data/Shared/Date/ExcelToTimestamp1904.php @@ -42,4 +42,9 @@ gmmktime(13, 02, 13, 1, 1, 1904), // 32-bit safe - no Y2038 problem 0.54321, ], + // 29-Apr-2038 00:00:00 beyond PHP 32-bit Latest Date + [ + 2156112000, + 49062, + ], ];