Skip to content

Commit 1fe9693

Browse files
oleibmanoleibman
authored
Xls Reader Handle MACCENTRALEUROPE With or Without Hyphen (PHPOffice#2213)
* Xls Reader Handle MACCENTRALEUROPE With or Without Hyphen Fixes issue PHPOffice#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. * Cache Successful Result For Future Use Per suggestion from @MarkBaker
1 parent 7c955b1 commit 1fe9693

File tree

6 files changed

+76
-21
lines changed

6 files changed

+76
-21
lines changed

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3490,11 +3490,6 @@ parameters:
34903490
count: 1
34913491
path: src/PhpSpreadsheet/Settings.php
34923492

3493-
-
3494-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\CodePage\\:\\:\\$pageArray has no typehint specified\\.$#"
3495-
count: 1
3496-
path: src/PhpSpreadsheet/Shared/CodePage.php
3497-
34983493
-
34993494
message: "#^Parameter \\#1 \\$dateValue of static method PhpOffice\\\\PhpSpreadsheet\\\\Shared\\\\Date\\:\\:timestampToExcel\\(\\) expects int, float\\|int\\|string given\\.$#"
35003495
count: 1
@@ -6525,16 +6520,6 @@ parameters:
65256520
count: 1
65266521
path: tests/PhpSpreadsheetTests/Reader/Security/XmlScannerTest.php
65276522

6528-
-
6529-
message: "#^Cannot call method getFormattedValue\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
6530-
count: 1
6531-
path: tests/PhpSpreadsheetTests/Reader/XlsTest.php
6532-
6533-
-
6534-
message: "#^Cannot call method getCoordinate\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
6535-
count: 1
6536-
path: tests/PhpSpreadsheetTests/Reader/XlsTest.php
6537-
65386523
-
65396524
message: "#^Method PhpOffice\\\\PhpSpreadsheetTests\\\\Reader\\\\Xlsx\\\\AutoFilterTest\\:\\:getWorksheetInstance\\(\\) has no return typehint specified\\.$#"
65406525
count: 1

‎src/PhpSpreadsheet/Shared/CodePage.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class CodePage
88
{
99
public const DEFAULT_CODE_PAGE = 'CP1252';
1010

11+
/** @var array */
1112
private static $pageArray = [
1213
0 => 'CP1252', // CodePage is not always correctly set when the xls file was saved by Apple's Numbers program
1314
367 => 'ASCII', // ASCII
@@ -56,7 +57,7 @@ class CodePage
5657
10010 => 'MACROMANIA', // Macintosh Romania
5758
10017 => 'MACUKRAINE', // Macintosh Ukraine
5859
10021 => 'MACTHAI', // Macintosh Thai
59-
10029 => 'MACCENTRALEUROPE', // Macintosh Central Europe
60+
10029 => ['MACCENTRALEUROPE', 'MAC-CENTRALEUROPE'], // Macintosh Central Europe
6061
10079 => 'MACICELAND', // Macintosh Icelandic
6162
10081 => 'MACTURKISH', // Macintosh Turkish
6263
10082 => 'MACCROATIAN', // Macintosh Croatian
@@ -65,6 +66,7 @@ class CodePage
6566
//32769 => 'unsupported', // ANSI Latin I (BIFF2-BIFF3)
6667
65000 => 'UTF-7', // Unicode (UTF-7)
6768
65001 => 'UTF-8', // Unicode (UTF-8)
69+
99999 => ['unsupported'], // Unicode (UTF-8)
6870
];
6971

7072
public static function validate(string $codePage): bool
@@ -83,7 +85,20 @@ public static function validate(string $codePage): bool
8385
public static function numberToName(int $codePage): string
8486
{
8587
if (array_key_exists($codePage, self::$pageArray)) {
86-
return self::$pageArray[$codePage];
88+
$value = self::$pageArray[$codePage];
89+
if (is_array($value)) {
90+
foreach ($value as $encoding) {
91+
if (@iconv('UTF-8', $encoding, ' ') !== false) {
92+
self::$pageArray[$codePage] = $encoding;
93+
94+
return $encoding;
95+
}
96+
}
97+
98+
throw new PhpSpreadsheetException("Code page $codePage not implemented on this system.");
99+
} else {
100+
return $value;
101+
}
87102
}
88103
if ($codePage == 720 || $codePage == 32769) {
89104
throw new PhpSpreadsheetException("Code page $codePage not supported."); // OEM Arabic

tests/PhpSpreadsheetTests/Reader/XlsTest.php renamed to tests/PhpSpreadsheetTests/Reader/Xls/XlsTest.php

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
<?php
22

3-
namespace PhpOffice\PhpSpreadsheetTests\Reader;
3+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xls;
44

5+
use PhpOffice\PhpSpreadsheet\Cell\Cell;
56
use PhpOffice\PhpSpreadsheet\Reader\Xls;
7+
use PhpOffice\PhpSpreadsheet\Shared\CodePage;
68
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
79

810
class XlsTest extends AbstractFunctional
@@ -16,6 +18,7 @@ public function testLoadXlsSample(): void
1618
$reader = new Xls();
1719
$spreadsheet = $reader->load($filename);
1820
self::assertEquals('Title', $spreadsheet->getSheet(0)->getCell('A1')->getValue());
21+
$spreadsheet->disconnectWorksheets();
1922
}
2023

2124
/**
@@ -42,6 +45,8 @@ public function testLoadXlsBug1505(): void
4245
self::assertEquals($row, $newrow);
4346
self::assertEquals($sheet->getCell('A1')->getFormattedValue(), $newsheet->getCell('A1')->getFormattedValue());
4447
self::assertEquals($sheet->getCell("$col$row")->getFormattedValue(), $newsheet->getCell("$col$row")->getFormattedValue());
48+
$spreadsheet->disconnectWorksheets();
49+
$newspreadsheet->disconnectWorksheets();
4550
}
4651

4752
/**
@@ -71,11 +76,49 @@ public function testLoadXlsBug1592(): void
7176
$rowIterator = $sheet->getRowIterator();
7277

7378
foreach ($rowIterator as $row) {
74-
foreach ($row->getCellIterator() as $cell) {
79+
foreach ($row->getCellIterator() as $cellx) {
80+
/** @var Cell */
81+
$cell = $cellx;
7582
$valOld = $cell->getFormattedValue();
7683
$valNew = $newsheet->getCell($cell->getCoordinate())->getFormattedValue();
7784
self::assertEquals($valOld, $valNew);
7885
}
7986
}
87+
$spreadsheet->disconnectWorksheets();
88+
$newspreadsheet->disconnectWorksheets();
89+
}
90+
91+
/**
92+
* Test load Xls file with MACCENTRALEUROPE encoding, which is implemented
93+
* as MAC-CENTRALEUROPE on some systems. Issue #549.
94+
*/
95+
public function testLoadMacCentralEurope(): void
96+
{
97+
$codePages = CodePage::getEncodings();
98+
self::assertIsArray($codePages[10029]);
99+
$filename = 'tests/data/Reader/XLS/maccentraleurope.xls';
100+
$reader = new Xls();
101+
// When no fix applied, spreadsheet fails to load on some systems
102+
$spreadsheet = $reader->load($filename);
103+
$sheet = $spreadsheet->getActiveSheet();
104+
self::assertSame('Ładowność', $sheet->getCell('I1')->getValue());
105+
$spreadsheet->disconnectWorksheets();
106+
}
107+
108+
/**
109+
* First test changes array entry in CodePage.
110+
* This test confirms new that new entry is okay.
111+
*/
112+
public function testLoadMacCentralEurope2(): void
113+
{
114+
$codePages = CodePage::getEncodings();
115+
self::assertIsString($codePages[10029]);
116+
$filename = 'tests/data/Reader/XLS/maccentraleurope.xls';
117+
$reader = new Xls();
118+
// When no fix applied, spreadsheet fails to load on some systems
119+
$spreadsheet = $reader->load($filename);
120+
$sheet = $spreadsheet->getActiveSheet();
121+
self::assertSame('Ładowność', $sheet->getCell('I1')->getValue());
122+
$spreadsheet->disconnectWorksheets();
80123
}
81124
}

‎tests/PhpSpreadsheetTests/Shared/CodePageTest.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,15 @@ class CodePageTest extends TestCase
1616
*/
1717
public function testCodePageNumberToName($expectedResult, $codePageIndex): void
1818
{
19+
if ($expectedResult === 'exception') {
20+
$this->expectException(Exception::class);
21+
}
1922
$result = CodePage::numberToName($codePageIndex);
20-
self::assertEquals($expectedResult, $result);
23+
if (is_array($expectedResult)) {
24+
self::assertContains($result, $expectedResult);
25+
} else {
26+
self::assertEquals($expectedResult, $result);
27+
}
2128
}
2229

2330
public function providerCodePage(): array

‎tests/data/Reader/XLS/maccentraleurope.xls‎

20 KB
Binary file not shown.

‎tests/data/Shared/CodePage.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@
233233
],
234234
// Macintosh Central Europe
235235
[
236-
'MACCENTRALEUROPE',
236+
['MACCENTRALEUROPE', 'MAC-CENTRALEUROPE'],
237237
10029,
238238
],
239239
// Macintosh Icelandic
@@ -271,4 +271,9 @@
271271
'UTF-8',
272272
65001,
273273
],
274+
// invalid
275+
[
276+
'exception',
277+
99999,
278+
],
274279
];

0 commit comments

Comments
 (0)