Read files with dangling references instead of panicking - #377
Open
keithadler wants to merge 1 commit into
Open
keithadler wants to merge 1 commit into
keithadler wants to merge 1 commit into
Conversation
Four references that point at nothing made the reader panic: - a defined name whose localSheetId is past the last sheet (`sheet_mut(..).unwrap()` in reader/xlsx/workbook.rs) - a hyperlink whose r:id names no relationship (`relationship_by_rid` panics when the id is not found) - a conditional format whose dxfId is past the differential formats (`DifferentialFormats::style` unwraps `get(id)`) - a comment whose authorId is missing or past the author list (`Comment::set_attributes` unwraps both the parse and `get`) Excel opens each of these and repairs it by dropping the dangling reference, so do the same: skip the name, keep the hyperlink without a URL, keep the rule without a style, and keep the comment without an author. `find_relationship_by_rid` and `DifferentialFormats::find_style` are the non-panicking lookups; the existing panicking ones are unchanged for their other callers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019hXzkiuCwk5QQRsyU9wT9h
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.
Problem
reader::xlsx::readpanics on four kinds of reference that point at nothing:localSheetIdpast the last sheetsheet_mut(..).unwrap()inreader/xlsx/workbook.rsr:idnames no relationshiprelationship_by_rid: "Not found relationship with ID"dxfIdis past the<dxfs>DifferentialFormats::style:get(id).unwrap()authorIdis missing or past the<authors>Comment::set_attributes:authors.get(..).unwrap()A panic in a reader takes down whatever called it, for example a server handling an uploaded file, where a
Resultcould have been handled.Change
Excel opens all four and repairs them by dropping the dangling reference, so this does the same:
RawRelationships::find_relationship_by_ridandDifferentialFormats::find_styleare new non-panicking lookups. The existing panicking ones are unchanged for their other callers.Tests
tests/dangling_references.rsbuilds each file from a workbook written by umya-spreadsheet itself, with one reference patched to dangle, and checks the read succeeds and what was kept. All four panic without the change.cargo +nightly fmt --all --check,cargo clippy -- -D warningsandcargo testpass.Related: #310 lists other
unwrap()panics on real files; #374 is the hyperlink-over-a-range panic, a different cause not addressed here. Found by the adversarial corpus of xlsx-lean, which writes one file per broken rule.🤖 Generated with Claude Code
https://claude.ai/code/session_019hXzkiuCwk5QQRsyU9wT9h