Skip to content

Commit

Permalink
Fix for #1505 (#1525)
Browse files Browse the repository at this point in the history
This problem is the same as #1238, which was resolved by #1239.
For that issue, the fix was to check in one place whether
$this->mapCellXfIndex[$xfIndex] was set before using it.
The sample spreadsheet supplied as a description for this
problem had exactly the same problem in 2 other places in the code.
In addition, there were 7 other places in the code where that
particular item was used unchecked. This fix corrects all 9 locations.
The spreadsheet supplied with the problem is used as the basis
for some new tests, which particularly test column dimensions
and styles, the problems involved in this case.
  • Loading branch information
oleibman committed Jun 19, 2020
1 parent 3844186 commit 38fab4e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 11 deletions.
22 changes: 13 additions & 9 deletions src/PhpSpreadsheet/Reader/Xls.php
Original file line number Diff line number Diff line change
Expand Up @@ -3622,7 +3622,9 @@ private function readColInfo(): void
$this->phpSheet->getColumnDimensionByColumn($i)->setVisible(!$isHidden);
$this->phpSheet->getColumnDimensionByColumn($i)->setOutlineLevel($level);
$this->phpSheet->getColumnDimensionByColumn($i)->setCollapsed($isCollapsed);
$this->phpSheet->getColumnDimensionByColumn($i)->setXfIndex($this->mapCellXfIndex[$xfIndex]);
if (isset($this->mapCellXfIndex[$xfIndex])) {
$this->phpSheet->getColumnDimensionByColumn($i)->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
}
}
}
Expand Down Expand Up @@ -3731,7 +3733,7 @@ private function readRk(): void
$numValue = self::getIEEE754($rknum);

$cell = $this->phpSheet->getCell($columnString . ($row + 1));
if (!$this->readDataOnly) {
if (!$this->readDataOnly && isset($this->mapCellXfIndex[$xfIndex])) {
// add style information
$cell->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
Expand Down Expand Up @@ -3866,7 +3868,7 @@ private function readMulRk(): void
// offset: var; size: 4; RK value
$numValue = self::getIEEE754(self::getInt4d($recordData, $offset + 2));
$cell = $this->phpSheet->getCell($columnString . ($row + 1));
if (!$this->readDataOnly) {
if (!$this->readDataOnly && isset($this->mapCellXfIndex[$xfIndex])) {
// add style
$cell->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
Expand Down Expand Up @@ -3910,7 +3912,7 @@ private function readNumber(): void
$numValue = self::extractNumber(substr($recordData, 6, 8));

$cell = $this->phpSheet->getCell($columnString . ($row + 1));
if (!$this->readDataOnly) {
if (!$this->readDataOnly && isset($this->mapCellXfIndex[$xfIndex])) {
// add cell style
$cell->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
Expand Down Expand Up @@ -4018,7 +4020,7 @@ private function readFormula(): void
}

$cell = $this->phpSheet->getCell($columnString . ($row + 1));
if (!$this->readDataOnly) {
if (!$this->readDataOnly && isset($this->mapCellXfIndex[$xfIndex])) {
// add cell style
$cell->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
Expand Down Expand Up @@ -4156,7 +4158,7 @@ private function readBoolErr(): void
break;
}

if (!$this->readDataOnly) {
if (!$this->readDataOnly && isset($this->mapCellXfIndex[$xfIndex])) {
// add cell style
$cell->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
Expand Down Expand Up @@ -4194,7 +4196,9 @@ private function readMulBlank(): void
// Read cell?
if (($this->getReadFilter() !== null) && $this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) {
$xfIndex = self::getUInt2d($recordData, 4 + 2 * $i);
$this->phpSheet->getCell($columnString . ($row + 1))->setXfIndex($this->mapCellXfIndex[$xfIndex]);
if (isset($this->mapCellXfIndex[$xfIndex])) {
$this->phpSheet->getCell($columnString . ($row + 1))->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
}
}
}
Expand Down Expand Up @@ -4245,7 +4249,7 @@ private function readLabel(): void
$cell = $this->phpSheet->getCell($columnString . ($row + 1));
$cell->setValueExplicit($value, DataType::TYPE_STRING);

if (!$this->readDataOnly) {
if (!$this->readDataOnly && isset($this->mapCellXfIndex[$xfIndex])) {
// add cell style
$cell->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
Expand Down Expand Up @@ -4277,7 +4281,7 @@ private function readBlank(): void
$xfIndex = self::getUInt2d($recordData, 4);

// add style information
if (!$this->readDataOnly && $this->readEmptyCells) {
if (!$this->readDataOnly && $this->readEmptyCells && isset($this->mapCellXfIndex[$xfIndex])) {
$this->phpSheet->getCell($columnString . ($row + 1))->setXfIndex($this->mapCellXfIndex[$xfIndex]);
}
}
Expand Down
30 changes: 28 additions & 2 deletions tests/PhpSpreadsheetTests/Reader/XlsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
namespace PhpOffice\PhpSpreadsheetTests\Reader;

use PhpOffice\PhpSpreadsheet\Reader\Xls;
use PHPUnit\Framework\TestCase;
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;

class XlsTest extends TestCase
class XlsTest extends AbstractFunctional
{
/**
* Test load Xls file.
Expand All @@ -17,4 +17,30 @@ public function testLoadXlsSample(): void
$spreadsheet = $reader->load($filename);
self::assertEquals('Title', $spreadsheet->getSheet(0)->getCell('A1')->getValue());
}

/**
* Test load Xls file with invalid xfIndex.
*/
public function testLoadXlsBug1505(): void
{
$filename = 'tests/data/Reader/XLS/bug1505.xls';
$reader = new Xls();
$spreadsheet = $reader->load($filename);
$sheet = $spreadsheet->getActiveSheet();
$col = $sheet->getHighestColumn();
$row = $sheet->getHighestRow();

$newspreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
$newsheet = $newspreadsheet->getActiveSheet();
$newcol = $newsheet->getHighestColumn();
$newrow = $newsheet->getHighestRow();

self::assertEquals($spreadsheet->getSheetCount(), $newspreadsheet->getSheetCount());
self::assertEquals($sheet->getTitle(), $newsheet->getTitle());
self::assertEquals($sheet->getColumnDimensions(), $newsheet->getColumnDimensions());
self::assertEquals($col, $newcol);
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());
}
}
Binary file added tests/data/Reader/XLS/bug1505.xls
Binary file not shown.

0 comments on commit 38fab4e

Please sign in to comment.