crate: return an error from Range::from_sparse instead of aborting - #697
Open
krickert wants to merge 3 commits into
Open
crate: return an error from Range::from_sparse instead of aborting#697krickert wants to merge 3 commits into
krickert wants to merge 3 commits into
Conversation
A Range is dense, so the extent implied by the cell positions decides the allocation, not how many cells were supplied. A sheet holding only A1 and XFD1048576 densifies to 17179869184 cells, and at 32 bytes per Data that is 549755813888 bytes. vec![T::default(); len] cannot report that failure: it calls handle_alloc_error, which aborts the process without unwinding. A consumer could not defend against it at all, since catch_unwind does not catch an abort and running the parse on its own thread does not contain it. The only stable-Rust workaround was to avoid the function. Reserving with try_reserve_exact turns that into a panic, which a caller can catch. On a 2 KB file that previously died with SIGABRT (exit 134), the process now survives with the caller in control. Also widens the extent arithmetic before the `+ 1`, so a range spanning a full u32 axis does not overflow it, which matters on wasm32 where usize is 32 bits. Files that load today are unaffected: the guard only fires where the allocation could not have been satisfied anyway. tests/issue_174.xlsx declares the entire grid but holds a handful of cells, densifies to 2x11 and still loads; there is now a test asserting that. Fixes tafia#693
Collaborator
jmcnamara
reviewed
Jul 29, 2026
jmcnamara
requested changes
Jul 29, 2026
jmcnamara
added
the
needs work for merge
krickert
changed the title
Author
|
from_sparse now returns a Result as requested, let me know what you think. |
krickert
added a commit
to ai-pipestream/calamine
that referenced
this pull request
Aug 20, 2026
Brings pipestream-main from 0.36.0 + the original fix commits to upstream/master at 0.36.1+ plus the post-review heads of the three open upstream PRs: tafia#695 dimension normalisation (Dimensions::new) tafia#696 grid-limit errors from cell reference parsing tafia#697 Range::from_sparse returns Result<Range<T>, RangeError> The tafia#697 head changes a public signature; the tree is byte-identical to upstream/master with the three fix branches merged (verified by diff), and the calamine test suite passes on that tree (46+164+65 tests).
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
2 participants
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.
Fixes #693.
A
Rangeis dense, so the extent implied by the outermost cell positions decides the allocation, not the number of cells. A sheet holding onlyA1andXFD1048576implies 17,179,869,184 cells, andvec![T::default(); len]aborts the process when that allocation fails, which a caller cannot catch.As requested in review,
from_sparsenow returnsResult<Range<T>, RangeError>. The buffer is reserved withtry_reserve_exact, and the error carries the implied width and height. The xls, xlsb and xlsx readers propagate it through a newRangevariant on their error enums, soworksheet_rangereturnsErrinstead of killing the process. The extent arithmetic is widened to u64 before the+ 1, which was a second overflow route on 32-bit targets.This changes the signature of a public function; the doc examples are updated to match.
Tests: an oversized two-corner extent returns the error with its dimensions,
tests/issue_693.xlsxexercises the same through the xlsx reader, and a sparse-but-large extent still loads, includingissue_174.xlsx, which declares the full grid but densifies to 2x11.