Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xls Reader Handle MACCENTRALEUROPE With or Without Hyphen #2213

Merged
merged 3 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
15 changes: 0 additions & 15 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3490,11 +3490,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 @@ -6525,16 +6520,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
19 changes: 17 additions & 2 deletions src/PhpSpreadsheet/Shared/CodePage.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class CodePage
{
public const DEFAULT_CODE_PAGE = 'CP1252';

/** @var array */
private static $pageArray = [
0 => 'CP1252', // CodePage is not always correctly set when the xls file was saved by Apple's Numbers program
367 => 'ASCII', // ASCII
Expand Down Expand Up @@ -56,7 +57,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,6 +66,7 @@ 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
Expand All @@ -83,7 +85,20 @@ 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];
$value = self::$pageArray[$codePage];
if (is_array($value)) {
foreach ($value as $encoding) {
if (@iconv('UTF-8', $encoding, ' ') !== false) {
oleibman marked this conversation as resolved.
Show resolved Hide resolved
self::$pageArray[$codePage] = $encoding;

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 Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
<?php

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

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

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

/**
Expand All @@ -42,6 +45,8 @@ public function testLoadXlsBug1505(): void
self::assertEquals($row, $newrow);
self::assertEquals($sheet->getCell('A1')->getFormattedValue(), $newsheet->getCell('A1')->getFormattedValue());
self::assertEquals($sheet->getCell("$col$row")->getFormattedValue(), $newsheet->getCell("$col$row")->getFormattedValue());
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}

/**
Expand Down Expand Up @@ -71,11 +76,49 @@ 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);
}
}
$spreadsheet->disconnectWorksheets();
$newspreadsheet->disconnectWorksheets();
}

/**
* Test load Xls file with MACCENTRALEUROPE encoding, which is implemented
* as MAC-CENTRALEUROPE on some systems. Issue #549.
*/
public function testLoadMacCentralEurope(): void
{
$codePages = CodePage::getEncodings();
self::assertIsArray($codePages[10029]);
$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());
$spreadsheet->disconnectWorksheets();
}

/**
* First test changes array entry in CodePage.
* This test confirms new that new entry is okay.
*/
public function testLoadMacCentralEurope2(): void
{
$codePages = CodePage::getEncodings();
self::assertIsString($codePages[10029]);
$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());
$spreadsheet->disconnectWorksheets();
}
}
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,
],
];