Skip to content

Commit 09fab17

Browse files
oleibmanoleibman
authored
Merge pull request #4687 from zlatanovic-nebojsa/fix-dimensions-record
Fix BIFF8 DIMENSIONS record to use 0-based column indices
2 parents eecfb67 + 21dc7fb commit 09fab17

File tree

3 files changed

+140
-10
lines changed

3 files changed

+140
-10
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

‎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;

‎tests/PhpSpreadsheetTests/Writer/Xls/DimensionsRecordTest.php‎

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)