Fix/oversized spreadsheet error php#700
Open
timseidel wants to merge 2 commits into
Open
Conversation
A stray styled/valued cell far down a sheet (typically the Excel max row A1048576) inflated getHighestDataRow() to ~1,048,576. The row iterator, running with setIterateOnlyExistingCells(false), then materialized a Cell per row/column until PHP exhausted its 512MB limit — a fatal memory error that surfaces deep in PhpSpreadsheet (Coordinate.php) and, being a fatal rather than a \Throwable, cannot be caught. readChoicesSheet() and readSurveySheet() now refuse a sheet reporting more than SpreadsheetReader::MAX_DATA_ROWS (50,000) rows with a clear, actionable error before iterating. This is the row-axis counterpart to the v1.1.0 column-axis fix for the same class of spreadsheet bloat. Also widen the catch in readItemTableFile() from PhpSpreadsheet\Exception to \Throwable so reader-internal TypeErrors degrade to a graceful error instead of a 500 (the memory fatal stays uncatchable — the row cap is its guard). Regression-tested in tests/SpreadsheetReaderRowCapTest.php. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When the choices sheet errored (e.g. the row-cap refusal), readItemTableFile() still ran readSurveySheet(). Its "if ($this->errors)" guard cannot tell errors it raises itself from ones carried over from the choices sheet, so it re-wrapped and re-threw the choices error on the very first survey cell. That throw was caught by the generic handler, producing a duplicate, misattributed cascade: 1. The choices worksheet "choices" reports 1,048,576 rows... (clean) 2. An error occured reading your excel file... (generic) 3. Error in cell B2 (Survey Sheet): <the choices error again> (duplicate) Short-circuit: skip readSurveySheet() when the choices sheet already failed — the survey sheet references those choice lists and is unreadable anyway. Leaves exactly the one clean error. Latent cascade for any choices-sheet error, exposed by the row-cap fix. Regression-tested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed a fatal php oom error which I encountered when uploading a spreadsheet where excel had added a formatted cell at the very bottom of the spreadsheet, leading to getHighestDataRow() reporting 1,048,576 rows (the Excel maximum). The row iterator, running with setIterateOnlyExistingCells(false), then materializes a Cell object per row/column until PHP exhausts its 512 MB memory limit. This fatal memory error surfaces deep in PhpSpreadsheet (Coordinate.php) and cannot be caught by any try/catch block.
Fix:
This is the row-axis counterpart to the v1.1.0 column-axis fix for the same class of spreadsheet bloat. Regression-tested in tests/SpreadsheetReaderRowCapTest.php (4 cases).