Skip to content

Commit 030ece8

Browse files
authored
Merge branch 'master' into listdimensions
2 parents 02090c1 + 6494ddb commit 030ece8

File tree

8 files changed

+179
-36
lines changed

8 files changed

+179
-36
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). Thia is a
4444
- Missing array key x for Xlsx Reader VML. [Issue #4505](https://github.com/PHPOffice/PhpSpreadsheet/issues/4505) [PR #4676](https://github.com/PHPOffice/PhpSpreadsheet/pull/4676)
4545
- Better support for Style Alignment Read Order. [Issue #850](https://github.com/PHPOffice/PhpSpreadsheet/issues/850) [PR #4655](https://github.com/PHPOffice/PhpSpreadsheet/pull/4655)
4646
- More sophisticated workbook password algorithms (Xlsx only). [Issue #4673](https://github.com/PHPOffice/PhpSpreadsheet/issues/4673) [PR #4675](https://github.com/PHPOffice/PhpSpreadsheet/pull/4675)
47+
- Xls Writer fix DIMENSIONS record. [Issue #4682](https://github.com/PHPOffice/PhpSpreadsheet/issues/4682) [PR #4687](https://github.com/PHPOffice/PhpSpreadsheet/pull/4687)
4748

4849
## 2025-09-03 - 5.1.0
4950

phpstan-baseline.neon

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,6 @@ parameters:
66
count: 1
77
path: src/PhpSpreadsheet/Calculation/LookupRef/Sort.php
88

9-
-
10-
message: '#^Cannot call method getAllSpContainers\(\) on mixed\.$#'
11-
identifier: method.nonObject
12-
count: 1
13-
path: src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php
14-
15-
-
16-
message: '#^Cannot call method getBSECollection\(\) on mixed\.$#'
17-
identifier: method.nonObject
18-
count: 1
19-
path: src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php
20-
21-
-
22-
message: '#^Cannot call method getBstoreContainer\(\) on mixed\.$#'
23-
identifier: method.nonObject
24-
count: 1
25-
path: src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php
26-
27-
-
28-
message: '#^Cannot call method getSpgrContainer\(\) on mixed\.$#'
29-
identifier: method.nonObject
30-
count: 1
31-
path: src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php
32-
339
-
3410
message: '#^Cannot access offset 0 on mixed\.$#'
3511
identifier: offsetAccess.nonOffsetAccessible

src/PhpSpreadsheet/Reader/Xls/Escher.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
use PhpOffice\PhpSpreadsheet\Shared\Escher\DggContainer\BstoreContainer\BSE;
1313
use PhpOffice\PhpSpreadsheet\Shared\Escher\DggContainer\BstoreContainer\BSE\Blip;
1414

15+
/**
16+
* @template T of BSE|BstoreContainer|DgContainer|DggContainer|\PhpOffice\PhpSpreadsheet\Shared\Escher|SpContainer|SpgrContainer
17+
*/
1518
class Escher
1619
{
1720
const DGGCONTAINER = 0xF000;
@@ -50,11 +53,15 @@ class Escher
5053

5154
/**
5255
* The object to be returned by the reader. Modified during load.
56+
*
57+
* @var T
5358
*/
5459
private BSE|BstoreContainer|DgContainer|DggContainer|\PhpOffice\PhpSpreadsheet\Shared\Escher|SpContainer|SpgrContainer $object;
5560

5661
/**
5762
* Create a new Escher instance.
63+
*
64+
* @param T $object
5865
*/
5966
public function __construct(BSE|BstoreContainer|DgContainer|DggContainer|\PhpOffice\PhpSpreadsheet\Shared\Escher|SpContainer|SpgrContainer $object)
6067
{
@@ -84,6 +91,8 @@ public function __construct(BSE|BstoreContainer|DgContainer|DggContainer|\PhpOff
8491

8592
/**
8693
* Load Escher stream data. May be a partial Escher stream.
94+
*
95+
* @return T
8796
*/
8897
public function load(string $data): BSE|BstoreContainer|DgContainer|DggContainer|\PhpOffice\PhpSpreadsheet\Shared\Escher|SpContainer|SpgrContainer
8998
{

src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ protected function loadSpreadsheetFromFile2(string $filename, Xls $xls): Spreads
435435

436436
// get all spContainers in one long array, so they can be mapped to OBJ records
437437
/** @var SpContainer[] $allSpContainers */
438-
$allSpContainers = method_exists($escherWorksheet, 'getDgContainer') ? $escherWorksheet->getDgContainer()->getSpgrContainer()->getAllSpContainers() : [];
438+
$allSpContainers = $escherWorksheet->getDgContainerOrThrow()->getSpgrContainerOrThrow()->getAllSpContainers();
439439
}
440440

441441
// treat OBJ records
@@ -497,7 +497,7 @@ protected function loadSpreadsheetFromFile2(string $filename, Xls $xls): Spreads
497497

498498
if ($escherWorkbook) {
499499
/** @var BSE[] */
500-
$BSECollection = method_exists($escherWorkbook, 'getDggContainer') ? $escherWorkbook->getDggContainer()->getBstoreContainer()->getBSECollection() : [];
500+
$BSECollection = $escherWorkbook->getDggContainerOrThrow()->getBstoreContainerOrThrow()->getBSECollection();
501501
$BSE = $BSECollection[$BSEindex - 1];
502502
$blipType = $BSE->getBlipType();
503503

src/PhpSpreadsheet/Shared/Escher.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Shared;
44

5+
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
6+
57
class Escher
68
{
79
/**
@@ -22,6 +24,14 @@ public function getDggContainer(): ?Escher\DggContainer
2224
return $this->dggContainer;
2325
}
2426

27+
/**
28+
* Get Drawing Group Container.
29+
*/
30+
public function getDggContainerOrThrow(): Escher\DggContainer
31+
{
32+
return $this->dggContainer ?? throw new SpreadsheetException('dggContainer is unexpectedly null');
33+
}
34+
2535
/**
2636
* Set Drawing Group Container.
2737
*/
@@ -38,6 +48,14 @@ public function getDgContainer(): ?Escher\DgContainer
3848
return $this->dgContainer;
3949
}
4050

51+
/**
52+
* Get Drawing Container.
53+
*/
54+
public function getDgContainerOrThrow(): Escher\DgContainer
55+
{
56+
return $this->dgContainer ?? throw new SpreadsheetException('dgContainer is unexpectedly null');
57+
}
58+
4159
/**
4260
* Set Drawing Container.
4361
*/

src/PhpSpreadsheet/Shared/Escher/DggContainer.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Shared\Escher;
44

5+
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
6+
57
class DggContainer
68
{
79
/**
@@ -94,6 +96,14 @@ public function getBstoreContainer(): ?DggContainer\BstoreContainer
9496
return $this->bstoreContainer;
9597
}
9698

99+
/**
100+
* Get BLIP Store Container.
101+
*/
102+
public function getBstoreContainerOrThrow(): DggContainer\BstoreContainer
103+
{
104+
return $this->bstoreContainer ?? throw new SpreadsheetException('bstoreContainer is unexpectedly null');
105+
}
106+
97107
/**
98108
* Set BLIP Store Container.
99109
*/

src/PhpSpreadsheet/Writer/Xls/Worksheet.php

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -211,15 +211,14 @@ public function __construct(int &$str_total, int &$str_unique, array &$str_table
211211
$maxC = $this->phpSheet->getHighestColumn();
212212

213213
// Determine lowest and highest column and row
214-
$this->firstRowIndex = $minR;
215-
$this->lastRowIndex = ($maxR > 65535) ? 65535 : $maxR;
216-
217-
$this->firstColumnIndex = Coordinate::columnIndexFromString($minC);
218-
$this->lastColumnIndex = Coordinate::columnIndexFromString($maxC);
219-
220-
if ($this->lastColumnIndex > 255) {
221-
$this->lastColumnIndex = 255;
222-
}
214+
// BIFF8 DIMENSIONS record requires 0-based indices for both rows and columns
215+
// Row methods return 1-based values (Excel UI), so subtract 1 to convert to 0-based
216+
$this->firstRowIndex = $minR - 1;
217+
$this->lastRowIndex = ($maxR > 65536) ? 65535 : ($maxR - 1);
218+
219+
// Column methods return 1-based values (columnIndexFromString('A') = 1), so subtract 1
220+
$this->firstColumnIndex = Coordinate::columnIndexFromString($minC) - 1;
221+
$this->lastColumnIndex = min(255, Coordinate::columnIndexFromString($maxC) - 1);
223222
$this->writerWorkbook = $writerWorkbook;
224223
}
225224

@@ -258,7 +257,8 @@ public function close(): void
258257
}
259258

260259
$columnDimensions = $phpSheet->getColumnDimensions();
261-
$maxCol = $this->lastColumnIndex - 1;
260+
// lastColumnIndex is now 0-based, so no need to subtract 1
261+
$maxCol = $this->lastColumnIndex;
262262
for ($i = 0; $i <= $maxCol; ++$i) {
263263
$hidden = 0;
264264
$level = 0;
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xls;
6+
7+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
8+
use PhpOffice\PhpSpreadsheet\Writer\Xls;
9+
use PHPUnit\Framework\TestCase;
10+
11+
class DimensionsRecordTest extends TestCase
12+
{
13+
/**
14+
* Test that DIMENSIONS record uses 0-based indices for both rows and columns.
15+
*
16+
* This test verifies that the BIFF8 DIMENSIONS record correctly uses 0-based
17+
* indices for both rows and columns. Prior to the fix, 1-based values were
18+
* written directly to the DIMENSIONS record, causing issues with old XLS parsers
19+
* that expect 0-based indices per the BIFF8 specification.
20+
*
21+
* The DIMENSIONS record structure (BIFF8):
22+
* - Offset 0-3: Index to first used row (0-based)
23+
* - Offset 4-7: Index to last used row + 1 (0-based)
24+
* - Offset 8-9: Index to first used column (0-based)
25+
* - Offset 10-11: Index to last used column + 1 (0-based)
26+
* - Offset 12-13: Not used
27+
*
28+
* Note: All indices in the DIMENSIONS record are 0-based, meaning Excel row 1
29+
* is stored as 0, row 5 as 4, column A as 0, column D as 3, etc.
30+
*/
31+
public function testDimensionsRecordUsesZeroBasedColumnIndices(): void
32+
{
33+
$spreadsheet = new Spreadsheet();
34+
$sheet = $spreadsheet->getActiveSheet();
35+
36+
// Set values in columns A through D (should be indices 0-3 in 0-based)
37+
$sheet->setCellValue('A1', 'Column A');
38+
$sheet->setCellValue('B1', 'Column B');
39+
$sheet->setCellValue('C1', 'Column C');
40+
$sheet->setCellValue('D1', 'Column D');
41+
$sheet->setCellValue('A5', 'Row 5');
42+
43+
// Write to XLS format
44+
$filename = tempnam(sys_get_temp_dir(), 'phpspreadsheet-test-');
45+
$writer = new Xls($spreadsheet);
46+
$writer->save($filename);
47+
$spreadsheet->disconnectWorksheets();
48+
49+
// Read the binary file and find the DIMENSIONS record
50+
$fileContent = file_get_contents($filename);
51+
self::assertIsString($fileContent, 'Failed to read XLS file');
52+
unlink($filename);
53+
54+
// Find the DIMENSIONS record: 0x0200 (2 bytes) + length 0x000E (2 bytes)
55+
$recordSignature = pack('v', 0x0200) . pack('v', 0x000E);
56+
$pos = strpos($fileContent, $recordSignature);
57+
58+
self::assertIsInt($pos, 'DIMENSIONS record not found in XLS file');
59+
60+
// Parse the DIMENSIONS record (skip 4-byte header)
61+
$dataPos = $pos + 4;
62+
$dimensionsData = substr($fileContent, $dataPos, 14);
63+
64+
// Unpack DIMENSIONS record
65+
$data = unpack('VrwMic/VrwMac/vcolMic/vcolMac/vreserved', $dimensionsData);
66+
self::assertIsArray($data, 'Failed to unpack DIMENSIONS record');
67+
68+
// Verify the values are correct (0-based for both rows and columns)
69+
// First used row is 1 (Excel UI), which is 0 in 0-based indexing
70+
self::assertSame(0, $data['rwMic'], 'First row should be 0 (0-based)');
71+
72+
// Last used row is 5 (Excel UI), which is 4 in 0-based, so rwMac should be 5 (4 + 1)
73+
self::assertSame(5, $data['rwMac'], 'Last row + 1 should be 5 (0-based row 4 + 1)');
74+
75+
// First column is A (Excel UI), which is 0 in 0-based indexing
76+
// BEFORE FIX: This would be 1 (because columnIndexFromString('A') returns 1)
77+
// AFTER FIX: This is 0 (because we subtract 1)
78+
self::assertSame(0, $data['colMic'], 'First column should be 0 (0-based for column A)');
79+
80+
// Last column is D (Excel UI), which is 3 in 0-based, so colMac should be 4 (3 + 1)
81+
// BEFORE FIX: This would be 5 (columnIndexFromString('D') = 4, not adjusted to 0-based)
82+
// AFTER FIX: This is 4 (columnIndexFromString('D') - 1 = 3, + 1 = 4)
83+
self::assertSame(4, $data['colMac'], 'Last column + 1 should be 4 (0-based column 3 + 1)');
84+
}
85+
86+
/**
87+
* Test that DIMENSIONS record correctly handles columns near the BIFF8 limit.
88+
*
89+
* BIFF8 format supports columns up to IV (256 columns, 0-based index 0-255).
90+
* This test ensures that the lastColumnIndex is correctly capped at 255.
91+
*/
92+
public function testDimensionsRecordCapsColumnIndexAt255(): void
93+
{
94+
$spreadsheet = new Spreadsheet();
95+
$sheet = $spreadsheet->getActiveSheet();
96+
97+
// Set value in column IV (column 256, 0-based index 255)
98+
$sheet->setCellValue('IV1', 'Last BIFF8 Column');
99+
100+
// Write to XLS format
101+
$filename = tempnam(sys_get_temp_dir(), 'phpspreadsheet-test-');
102+
$writer = new Xls($spreadsheet);
103+
$writer->save($filename);
104+
$spreadsheet->disconnectWorksheets();
105+
106+
// Read the binary file and find the DIMENSIONS record
107+
$fileContent = file_get_contents($filename);
108+
self::assertIsString($fileContent, 'Failed to read XLS file');
109+
unlink($filename);
110+
111+
// Find the DIMENSIONS record: 0x0200 (2 bytes) + length 0x000E (2 bytes)
112+
$recordSignature = pack('v', 0x0200) . pack('v', 0x000E);
113+
$pos = strpos($fileContent, $recordSignature);
114+
115+
self::assertIsInt($pos, 'DIMENSIONS record not found in XLS file');
116+
117+
// Parse the DIMENSIONS record (skip 4-byte header)
118+
$dataPos = $pos + 4;
119+
$dimensionsData = substr($fileContent, $dataPos, 14);
120+
121+
// Unpack DIMENSIONS record
122+
$data = unpack('VrwMic/VrwMac/vcolMic/vcolMac/vreserved', $dimensionsData);
123+
self::assertIsArray($data, 'Failed to unpack DIMENSIONS record');
124+
125+
// Last column should be capped at 256 (255 + 1 for "last used + 1")
126+
// The min(255, ...) ensures we don't exceed the BIFF8 limit
127+
self::assertLessThanOrEqual(256, $data['colMac'], 'Last column index should not exceed 256');
128+
}
129+
}

0 commit comments

Comments
 (0)