Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Xls Reader Handle MACCENTRALEUROPE With or Without Hyphen
Fixes issue #549 and SpartnerNL/Laravel-Excel#989 (which is the source of the new test file). Some systems accept MACCENTRALEUROPE as the name for the appropriate encoding, and some accept MAC-CENTRALEUROPE. I fortunately have access to at least one of each type, and have run the tests on each.

CodePage.php has an array of translations from codepage number to string. I now allow the value to itself be an array; if so, the code will test each in turn to see if it can be used in iconv. I did not go fishing for other similar problems. If such show up, they can be dealt with in the same manner as this one. I don't really expect others, since this is a problem not merely for Xls, but, even then, it applies only to BIFF5 and earlier.

I also moved XlsTest from Reader to Reader/Xls.
  • Loading branch information
oleibman committed Jul 7, 2021
commit 34eaecf30ddff69d2a7a0144c813d691d4265705
15 changes: 0 additions & 15 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3775,11 +3775,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Settings.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\CodePage\\:\\:\\$pageArray has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Shared/CodePage.php

-
message: "#^Parameter \\#1 \\$dateValue of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Date\\:\\:timestampToExcel\\(\\) expects int, float\\|int\\|string given\\.$#"
count: 1
Expand Down Expand Up @@ -6810,16 +6805,6 @@ parameters:
count: 1
path: tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php

-
message: "#^Cannot call method getFormattedValue\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsTest.php

-
message: "#^Cannot call method getCoordinate\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsTest.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheetTests\\\\Reader\\\\Xlsx\\\\AutoFilterTest\\:\\:getWorksheetInstance\\(\\) has no return typehint specified\\.$#"
count: 1
Expand Down
24 changes: 18 additions & 6 deletions src/PhpSpreadsheet/Shared/CodePage.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class CodePage
{
public const DEFAULT_CODE_PAGE = 'CP1252';

private static $pageArray = [
private const PAGE_ARRAY = [
0 => 'CP1252', // CodePage is not always correctly set when the xls file was saved by Apple's Numbers program
367 => 'ASCII', // ASCII
437 => 'CP437', // OEM US
Expand Down Expand Up @@ -56,7 +56,7 @@ class CodePage
10010 => 'MACROMANIA', // Macintosh Romania
10017 => 'MACUKRAINE', // Macintosh Ukraine
10021 => 'MACTHAI', // Macintosh Thai
10029 => 'MACCENTRALEUROPE', // Macintosh Central Europe
10029 => ['MACCENTRALEUROPE', 'MAC-CENTRALEUROPE'], // Macintosh Central Europe
10079 => 'MACICELAND', // Macintosh Icelandic
10081 => 'MACTURKISH', // Macintosh Turkish
10082 => 'MACCROATIAN', // Macintosh Croatian
Expand All @@ -65,11 +65,12 @@ class CodePage
//32769 => 'unsupported', // ANSI Latin I (BIFF2-BIFF3)
65000 => 'UTF-7', // Unicode (UTF-7)
65001 => 'UTF-8', // Unicode (UTF-8)
99999 => ['unsupported'], // Unicode (UTF-8)
];

public static function validate(string $codePage): bool
{
return in_array($codePage, self::$pageArray, true);
return in_array($codePage, self::PAGE_ARRAY, true);
}

/**
Expand All @@ -82,8 +83,19 @@ public static function validate(string $codePage): bool
*/
public static function numberToName(int $codePage): string
{
if (array_key_exists($codePage, self::$pageArray)) {
return self::$pageArray[$codePage];
if (array_key_exists($codePage, self::PAGE_ARRAY)) {
$value = self::PAGE_ARRAY[$codePage];
if (is_array($value)) {
foreach ($value as $encoding) {
if (@iconv('UTF-8', $encoding, ' ') !== false) {
return $encoding;
}
}

throw new PhpSpreadsheetException("Code page $codePage not implemented on this system.");
} else {
return $value;
}
}
if ($codePage == 720 || $codePage == 32769) {
throw new PhpSpreadsheetException("Code page $codePage not supported."); // OEM Arabic
Expand All @@ -94,6 +106,6 @@ public static function numberToName(int $codePage): string

public static function getEncodings(): array
{
return self::$pageArray;
return self::PAGE_ARRAY;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Reader;
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xls;

use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Reader\Xls;
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;

Expand Down Expand Up @@ -71,11 +72,27 @@ public function testLoadXlsBug1592(): void
$rowIterator = $sheet->getRowIterator();

foreach ($rowIterator as $row) {
foreach ($row->getCellIterator() as $cell) {
foreach ($row->getCellIterator() as $cellx) {
/** @var Cell */
$cell = $cellx;
$valOld = $cell->getFormattedValue();
$valNew = $newsheet->getCell($cell->getCoordinate())->getFormattedValue();
self::assertEquals($valOld, $valNew);
}
}
}

/**
* Test load Xls file with MACCENTRALEUROPE encoding, which is implemented
* as MAC-CENTRALEUROPE on some systems. Issue #549.
*/
public function testLoadMacCentralEurope(): void
{
$filename = 'tests/data/Reader/XLS/maccentraleurope.xls';
$reader = new Xls();
// When no fix applied, spreadsheet fails to load on some systems
$spreadsheet = $reader->load($filename);
$sheet = $spreadsheet->getActiveSheet();
self::assertSame('Ładowność', $sheet->getCell('I1')->getValue());
}
}
9 changes: 8 additions & 1 deletion tests/PhpSpreadsheetTests/Shared/CodePageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@ class CodePageTest extends TestCase
*/
public function testCodePageNumberToName($expectedResult, $codePageIndex): void
{
if ($expectedResult === 'exception') {
$this->expectException(Exception::class);
}
$result = CodePage::numberToName($codePageIndex);
self::assertEquals($expectedResult, $result);
if (is_array($expectedResult)) {
self::assertContains($result, $expectedResult);
} else {
self::assertEquals($expectedResult, $result);
}
}

public function providerCodePage(): array
Expand Down
Binary file added tests/data/Reader/XLS/maccentraleurope.xls
Binary file not shown.
7 changes: 6 additions & 1 deletion tests/data/Shared/CodePage.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@
],
// Macintosh Central Europe
[
'MACCENTRALEUROPE',
['MACCENTRALEUROPE', 'MAC-CENTRALEUROPE'],
10029,
],
// Macintosh Icelandic
Expand Down Expand Up @@ -271,4 +271,9 @@
'UTF-8',
65001,
],
// invalid
[
'exception',
99999,
],
];