The initial fix only converted column indices to 0-based, but overlooked
that row indices also need the same treatment per BIFF8 specification.
Changes:
- Convert firstRowIndex from 1-based to 0-based (subtract 1)
- Convert lastRowIndex from 1-based to 0-based (subtract 1)
- Update row capping logic for 65536 limit
- Fix test assertions to expect rwMic=0 and rwMac=5 (not 1 and 6)
- Enhanced documentation to clarify all DIMENSIONS indices are 0-based
This now matches the behavior observed in Excel-generated XLS files,
where a file with 3 rows × 5 columns shows: rwMic=0, rwMac=3, colMic=0, colMac=5
All tests pass (53 tests, 244 assertions).
Add proper type assertions to handle potential false returns:
- Assert file_get_contents() returns string, not false
- Assert strpos() returns int, not false
- Assert unpack() returns array, not false
This resolves all 7 PHPStan errors reported in CI while maintaining
test functionality (12 assertions, all passing).
This commit refines the BIFF8 DIMENSIONS record fix by:
1. **Optimized column index capping**: Replaced the if-statement with min()
function to ensure lastColumnIndex never exceeds 255, improving code
coverage and eliminating unreachable branches in unit tests.
2. **Added comprehensive unit tests**: Created DimensionsRecordTest.php which
directly parses the binary DIMENSIONS record (0x0200) from XLS files to
verify correct 0-based column indices.
The tests validate:
- colMic (first column) = 0 for column A (was incorrectly 1 before fix)
- colMac (last column + 1) uses proper 0-based indexing
- Column indices are correctly capped at 255 (BIFF8 limit)
These tests fail without the fix and pass with it, ensuring the DIMENSIONS
record is correctly written for compatibility with legacy XLS parsers that
expect 0-based column indices per the BIFF8 specification.
The XLS writer incorrectly used 1-based column indices in the BIFF8
DIMENSIONS record, violating the Microsoft Excel Binary File Format
specification which requires 0-based indices.
This bug caused an extra empty column to appear when converting XLS
files to other formats (e.g., CSV).
Changes:
- Modified column index initialization to subtract 1 from the result
of Coordinate::columnIndexFromString() to convert from 1-based to
0-based indexing
- Updated COLINFO loop to use the corrected 0-based lastColumnIndex
Per BIFF8 specification:
- colMic (first column) must be 0-based
- colMac (column after last column) must be 0-based
Example: For columns A-G (7 columns):
- Before: colMic=1, colMac=8 (incorrect)
- After: colMic=0, colMac=7 (correct)
Fixes#4682
Fix#4673. Our password hasher can handle different algorithms, but the workbook password (for maintaining the structure of the workbook, not for encrypting the entire workbook) currently supports only the single algorithm that was in place many years ago. Expand it, and Xlsx Reader and Writer, to be able to use, say, SHA-512, which is what Excel itself uses.
The revisions password needs to be expanded in the same way as the workbook password. It is used for file sharing, but MS has deprecated it because it feels that modern technologies introduce better ways to accomplish what it was needed for. You'll need to look pretty hard to even find it in Excel - it's not, for example, on any of the ribbons. Nevertheless, the solution here is pretty much identical to the solution for the workbook password, so I am fixing it at the same time.
By default, TCPDF will die sometimes rather than throwing exception. And this is controlled by a defined constant in the global namespace, not by an instance property. Ugh! Using this class instead of the class which it extends will probably be suitable for most users. But not for those who have customized their config file. Which is why this isn't the default, so that there is no breaking change for those users. Note that if both TCPDF and TcpdfNoDie are used in the same process, the first one used "wins" the battle of the defines.
Coveralls recommends using an "official integration" of their product with Github. Also, allow the upload to fail without causing the whole coverage step to fail - there have been some recent problems which they are still working on. I may allow the step to fail once they are done with their changes.
Fix#850 (marked stale many years ago, but now reopened). User had a typo in their script which would have caused problems no matter what. However, it exposed another problem. Style Alignment Read Order was supported only by the Xlsx Reader and Writer, but it could have been supported pretty easily for most other formats. This PR adds support for the following:
- Xls (read and write)
- Html (write, and read using inline styles). Html reader does not yet process most classes.
- Pdf (write). PhpSpreadsheet does not have a Pdf reader.
- Ods (write). PhpSpreadsheet does not yet support reading most Ods styles.
- Xml (read). PhpSpreadsheet does not have an Xml writer.
- Gnumeric (no change). It appears that the Gnumeric product does not support this attribute.
- Csv (no change). Csv does not support any styles.
- Slk (no change). Slk does not support non-Latin characters, so this attribute doesn't make sense for it.
Intl is only a "suggested" extension. A lot of the NumberFormat Wizard code depends on it. That's insufficient reason to make it required, but the suggestion text now mentions this dependency explicitly. Also clean up the Wizard documentation to reflect some changes since PhpSpreadsheet 1.28.
oleibman
Nebojša Zlatanović
karrakoliko
Emeka Oziri
dependabot[bot]