diff --git a/src/PhpSpreadsheet/Reader/Gnumeric.php b/src/PhpSpreadsheet/Reader/Gnumeric.php index 849c549ba..3947aea9e 100644 --- a/src/PhpSpreadsheet/Reader/Gnumeric.php +++ b/src/PhpSpreadsheet/Reader/Gnumeric.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Reader; +use PhpOffice\PhpSpreadsheet\Cell\AddressRange; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\DefinedName; @@ -472,11 +473,13 @@ class Gnumeric extends BaseReader $rowHeight = (float) $rowAttributes['Unit']; $hidden = (isset($rowAttributes['Hidden'])) && ((string) $rowAttributes['Hidden'] == '1'); $rowCount = (int) ($rowAttributes['Count'] ?? 1); - while ($whichRow < $row) { + $limit = min($row, AddressRange::MAX_ROW); + while ($whichRow < $limit) { ++$whichRow; $this->setRowHeight($whichRow, $defaultHeight); } - while (($whichRow < ($row + $rowCount)) && ($whichRow < $maxRow)) { + $limit = min($maxRow, AddressRange::MAX_ROW); + while (($whichRow < ($row + $rowCount)) && ($whichRow < $limit)) { ++$whichRow; $this->setRowHeight($whichRow, $rowHeight); if ($hidden) { diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index aa5e98c79..8c7f3669d 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Reader; +use Composer\Pcre\Preg; use DOMAttr; use DOMDocument; use DOMElement; @@ -258,13 +259,22 @@ class Ods extends BaseReader public function loadIntoExisting(string $filename, Spreadsheet $spreadsheet): Spreadsheet { File::assertFile($filename, self::INITIAL_FILE); - $zip = new ZipArchive(); - $zip->open($filename); + try { + $zip->open($filename); + + return $this->load2($filename, $spreadsheet, $zip); + } finally { + $zip->close(); + } + } + + public function load2(string $filename, Spreadsheet $spreadsheet, ZipArchive $zip): Spreadsheet + { $xml = @$this->loadZipSimple($zip, 'meta.xml'); if ($xml === false) { - throw new Exception('Unable to read data from {$pFilename}'); + throw new Exception('Unable to read data from {$filename}'); } $namespacesMeta = $xml->getNamespaces(true); @@ -449,16 +459,20 @@ class Ods extends BaseReader if ($cellData->hasAttributeNS($tableNs, 'number-matrix-columns-spanned')) { if ($cellData->hasAttributeNS($tableNs, 'number-matrix-rows-spanned')) { $cellDataType = 'array'; - $arrayRow = (int) $cellData->getAttributeNS($tableNs, 'number-matrix-rows-spanned'); - $arrayCol = (int) $cellData->getAttributeNS($tableNs, 'number-matrix-columns-spanned'); + $arrayRow = self::boundedInteger( + $cellData->getAttributeNS($tableNs, 'number-matrix-rows-spanned'), + 1, + AddressRange::MAX_ROW - $rowID + 1, + 'number-matrix-rows-spanned' + ); + $arrayCol = self::boundedInteger( + $cellData->getAttributeNS($tableNs, 'number-matrix-columns-spanned'), + 1, + AddressRange::MAX_COLUMN_INT - Coordinate::columnIndexFromString($columnID) + 1, + 'number-matrix-columns-spanned' + ); $lastRow = $rowID + $arrayRow - 1; - $lastCol = $columnID; - while ($arrayCol > 1) { - StringHelper::stringIncrement( - $lastCol - ); - --$arrayCol; - } + $lastCol = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($columnID) + $arrayCol - 1); $cellDataRef = "$columnID$rowID:$lastCol$lastRow"; } } @@ -701,8 +715,6 @@ class Ods extends BaseReader if ($zip->locateName('settings.xml') !== false) { $this->processSettings($zip, $spreadsheet); } - $zip->close(); - unset($this->zip); // Return return $spreadsheet; @@ -802,13 +814,13 @@ class Ods extends BaseReader } } - return $str; + return mb_substr($str, 0, DataType::MAX_STRING_LENGTH); } private static function getMultiplier(?DOMAttr $cAttr): int { if ($cAttr) { - $multiplier = (int) $cAttr->nodeValue; + $multiplier = max(1, min((int) $cAttr->nodeValue, DataType::MAX_STRING_LENGTH)); } else { $multiplier = 1; } @@ -963,4 +975,27 @@ class Ods extends BaseReader return (bool) $retVal; } + + /** Validate the lexical value and its bound before converting to an integer. */ + private static function boundedInteger(string $value, int $minimum, int $maximum, string $attribute): int + { + $value = trim($value); + if (!Preg::isMatch('/^\+?[0-9]+$/D', $value)) { + throw new Exception("Invalid ODS $attribute value."); + } + $digits = ltrim($value, '+0'); + $maximumDigits = (string) $maximum; + if ( + strlen($digits) > strlen($maximumDigits) + || (strlen($digits) === strlen($maximumDigits) && strcmp($digits, $maximumDigits) > 0) + ) { + throw new Exception("Invalid ODS $attribute value."); + } + $integer = (int) $value; + if ($integer < $minimum) { + throw new Exception("Invalid ODS $attribute value."); + } + + return $integer; + } } diff --git a/tests/PhpSpreadsheetTests/Reader/Gnumeric/Secf5w4Test.php b/tests/PhpSpreadsheetTests/Reader/Gnumeric/Secf5w4Test.php new file mode 100644 index 000000000..4d4b48b1d --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Gnumeric/Secf5w4Test.php @@ -0,0 +1,31 @@ +load($filename); + $sheet = $spreadsheet->getActiveSheet(); + $row = AddressRange::MAX_ROW; + self::assertTrue($sheet->rowDimensionExists($row)); + $rowDimension = $sheet->getRowDimension($row); + self::assertEqualsWithDelta(12.0, $rowDimension->getRowHeight(), 1.0e-8); + ++$row; + self::assertFalse($sheet->rowDimensionExists($row)); + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/LongStringTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/LongStringTest.php new file mode 100644 index 000000000..1ff40aff1 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/LongStringTest.php @@ -0,0 +1,24 @@ +load($infile); + $sheet = $spreadsheet->getActiveSheet(); + $value = $sheet->getCell('A1')->getValue(); + self::assertIsString($value); + self::assertSame(DataType::MAX_STRING_LENGTH, mb_strlen($value)); + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/MetadataBoundsShortTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/MetadataBoundsShortTest.php new file mode 100644 index 000000000..4960e03d6 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/MetadataBoundsShortTest.php @@ -0,0 +1,142 @@ + */ + private array $files = []; + + protected function tearDown(): void + { + foreach ($this->files as $file) { + unlink($file); + } + } + + public function testReaderRecoversAfterRejectedMatrixSpan(): void + { + $reader = new Ods(); + + try { + $reader->load($this->matrixFixture('2', '1', AddressRange::MAX_COLUMN_INT, 1)); + self::fail('Expected invalid matrix span rejection'); + } catch (ReaderException $exception) { + self::assertSame('Invalid ODS number-matrix-columns-spanned value.', $exception->getMessage()); + } + $spreadsheet = $reader->load($this->matrixFixture('3', '2', 2, 2)); + + try { + $cell = $spreadsheet->getActiveSheet()->getCell('B2'); + self::assertSame('=1', $cell->getValue()); + self::assertSame(['t' => 'array', 'ref' => 'B2:D3'], $cell->getFormulaAttributes()); + self::assertSame(1, $cell->getOldCalculatedValue()); + } finally { + $spreadsheet->disconnectWorksheets(); + } + } + + /** @return array */ + public static function invalidMatrices(): array + { + $cases = []; + foreach (['0', '-1', '', '1.5', 'abc', '2e9', '2000000000', '999999999999999999999999999999'] as $count) { + $cases['columns ' . $count] = [$count, '1', 1, 1, 'number-matrix-columns-spanned']; + $cases['rows ' . $count] = ['1', $count, 1, 1, 'number-matrix-rows-spanned']; + } + $cases['column past maximum'] = [(string) (AddressRange::MAX_COLUMN_INT + 1), '1', 1, 1, 'number-matrix-columns-spanned']; + $cases['row past maximum'] = ['1', (string) (AddressRange::MAX_ROW + 1), 1, 1, 'number-matrix-rows-spanned']; + $cases['columns from B exceed remaining width'] = [(string) AddressRange::MAX_COLUMN_INT, '1', 2, 1, 'number-matrix-columns-spanned']; + $cases['rows from row 2 exceed remaining height'] = ['1', (string) AddressRange::MAX_ROW, 1, 2, 'number-matrix-rows-spanned']; + $cases['columns from XFD exceed remaining width'] = ['2', '1', AddressRange::MAX_COLUMN_INT, 1, 'number-matrix-columns-spanned']; + $cases['rows from final row exceed remaining height'] = ['1', '2', 1, AddressRange::MAX_ROW, 'number-matrix-rows-spanned']; + + return $cases; + } + + #[DataProvider('invalidMatrices')] + public function testMatrixSpansAreRejectedBeforeIteration(string $columns, string $rows, int $column, int $row, string $attribute): void + { + $this->expectException(ReaderException::class); + $this->expectExceptionMessage('Invalid ODS ' . $attribute . ' value.'); + (new Ods())->load($this->matrixFixture($columns, $rows, $column, $row)); + } + + /** @return array */ + public static function validMatrices(): array + { + return [ + 'single cell' => ['1', '1', 1, 1, 'A1:A1'], + 'small matrix' => ['3', '2', 2, 2, 'B2:D3'], + 'integer lexical forms' => [' +003 ', '0002', 2, 2, 'B2:D3'], + 'full column width' => [(string) AddressRange::MAX_COLUMN_INT, '1', 1, 1, 'A1:XFD1'], + 'full row height' => ['1', (string) AddressRange::MAX_ROW, 1, 1, 'A1:A1048576'], + 'remaining width from B' => [(string) (AddressRange::MAX_COLUMN_INT - 1), '1', 2, 1, 'B1:XFD1'], + 'remaining height from row 2' => ['1', (string) (AddressRange::MAX_ROW - 1), 1, 2, 'A2:A1048576'], + 'bottom right cell' => ['1', '1', AddressRange::MAX_COLUMN_INT, AddressRange::MAX_ROW, 'XFD1048576:XFD1048576'], + ]; + } + + #[DataProvider('validMatrices')] + public function testValidMatrixRangesArePreserved(string $columns, string $rows, int $column, int $row, string $expected): void + { + $spreadsheet = (new Ods())->load($this->matrixFixture($columns, $rows, $column, $row)); + + try { + $start = explode(':', $expected)[0]; + $cell = $spreadsheet->getActiveSheet()->getCell($start); + self::assertSame(['t' => 'array', 'ref' => $expected], $cell->getFormulaAttributes()); + self::assertSame('=1', $cell->getValue()); + } finally { + $spreadsheet->disconnectWorksheets(); + } + } + + private function matrixFixture(string $columns, string $rows, int $column, int $row): string + { + $body = ''; + if ($row > 1) { + $body .= ''; + } + $body .= ''; + if ($column > 1) { + $body .= ''; + } + $body .= '1'; + + return $this->fixture($body); + } + + private function fixture(string $body, string $styles = '', string $automaticStyles = ''): string + { + $file = tempnam(sys_get_temp_dir(), 'ods-bounds-'); + self::assertNotFalse($file); + $this->files[] = $file; + $zip = new ZipArchive(); + self::assertTrue($zip->open($file, ZipArchive::OVERWRITE)); + $ns = 'xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" ' + . 'xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0" ' + . 'xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" ' + . 'xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" ' + . 'xmlns:number="urn:oasis:names:tc:opendocument:xmlns:datastyle:1.0" ' + . 'xmlns:of="urn:oasis:names:tc:opendocument:xmlns:of:1.2" ' + . 'xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" ' + . 'xmlns:xlink="http://www.w3.org/1999/xlink"'; + $zip->addFromString('mimetype', 'application/vnd.oasis.opendocument.spreadsheet'); + $zip->addFromString('meta.xml', ''); + $zip->addFromString('styles.xml', '' . $styles . ''); + $zip->addFromString('content.xml', '' . $automaticStyles . '' . $body . ''); + self::assertTrue($zip->close()); + + return $file; + } +} diff --git a/tests/data/Reader/Gnumeric/secf5w4.gnumeric b/tests/data/Reader/Gnumeric/secf5w4.gnumeric new file mode 100644 index 000000000..cb2c003eb --- /dev/null +++ b/tests/data/Reader/Gnumeric/secf5w4.gnumeric @@ -0,0 +1,17 @@ + + + + Sheet1 + + + + Sheet1 + 1 + 1 + + + + + + + diff --git a/tests/data/Reader/Ods/long.string b/tests/data/Reader/Ods/long.string new file mode 100644 index 000000000..71d3a6e8f Binary files /dev/null and b/tests/data/Reader/Ods/long.string differ