Conversation
skytin1004
left a comment
There was a problem hiding this comment.
Reviewed and verified locally.
The focused tests pass locally (4 tests). I also temporarily reverted the handler change and confirmed that the end-to-end test reproduces the original NumberFormatException in CellTagHandler.
The fix is narrowly scoped, and the regression coverage verifies both the internal handler and the public read path. I also confirmed that valid numeric style indexes are still handled through the existing lookup path, so the change does not affect the normal style lookup behavior.
LGTM.
|
POI doesn't actually provide fault tolerance for this scenario during streaming reads: Looking at the source code of Therefore, mentioning "matching POI's lenient behavior" in the comment might not be entirely accurate. In addition, while I agree that we should be lenient with non-standard files missing |
Fixes #355
What changes were proposed in this pull request?
CellTagHandlerparses a cell'ssattribute (style index) with a bareInteger.parseInt. A non-numeric value — as produced by some third-party tools — threwNumberFormatExceptionand aborted the whole file read at the first corrupt cell (in the report: reading stopped at row 3218 of 3000+ rows).The parse now falls back to the default format index (
0) onNumberFormatException: a corrupt style index only loses that cell's style info, while the cell value and all subsequent rows are still read. This follows the reporter's suggestion and matches POI's lenient behavior (POI reads the same file completely, with the corrupt cell read as empty).How was this patch tested?
CellTagHandlerTest(2 new unit tests): non-numericsfalls back todataFormatData(0)without throwing; numericskeeps the exact lookup.CorruptStyleIndexReadTest(new end-to-end test): a minimal hand-built xlsx whose middle row carriess="abc"is read completely — all 3 rows returned.NumberFormatException: For input string: "abc"); with the fix it passes.fesod-sheetsuite: 915/915 green.spotless:checkandjavadocpass.Notes