Skip to content

Commit

Permalink
Fix Two 32-bit Timestamp Problems, and Minor getFormattedValue Bug (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
oleibman committed Mar 3, 2021
1 parent c55269c commit 04e7c30
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/PhpSpreadsheet/Reader/Xml.php
Expand Up @@ -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;
Expand Down Expand Up @@ -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':
Expand Down
4 changes: 4 additions & 0 deletions src/PhpSpreadsheet/Style/NumberFormat.php
Expand Up @@ -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);
}

Expand Down
4 changes: 4 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xml/XmlLoadTest.php
Expand Up @@ -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());
Expand Down
13 changes: 13 additions & 0 deletions tests/PhpSpreadsheetTests/Shared/DateTest.php
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions tests/data/Shared/Date/ExcelToTimestamp1900.php
Expand Up @@ -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
];
5 changes: 5 additions & 0 deletions tests/data/Shared/Date/ExcelToTimestamp1904.php
Expand Up @@ -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,
],
];

0 comments on commit 04e7c30

Please sign in to comment.