Skip to content

fix(xls): ensure BIFF5 Lbl and ExternSheet records are parsed correctly#613

Merged
jmcnamara merged 1 commit intotafia:masterfrom
alexander-beedie:fix-biff5
Mar 6, 2026
Merged

fix(xls): ensure BIFF5 Lbl and ExternSheet records are parsed correctly#613
jmcnamara merged 1 commit intotafia:masterfrom
alexander-beedie:fix-biff5

Conversation

@alexander-beedie
Copy link
Contributor

Fixes #612.

  • Factored out some small record-parsing helpers to keep the code clean, and ensured there are BIFF5 parsing paths for the Lbl and ExternSheet record types (the error was caused by the code assuming BIFF8 byte layouts).
  • While staring at the byte identifiers I also added a few missing comments/section labels for future reference.
  • Added a unit test that validates the fix, using the sample workbook provided in the original issue 👍

@alexander-beedie alexander-beedie changed the title fix(xls): ensure BIFF5 Lbl and ExternSheet records are not parsed as BIFF8 fix(xls): ensure BIFF5 Lbl and ExternSheet records are parsed correctly Mar 2, 2026
@alexander-beedie alexander-beedie force-pushed the fix-biff5 branch 2 times, most recently from d579531 to 934c1f7 Compare March 2, 2026 07:16
@jmcnamara
Copy link
Collaborator

Overall looks good. It is a good tidy-up as well as a fix. I'll do some additional testing and merge it is a couple of days.

Ping @sftse in case you want to review too.

@alexander-beedie alexander-beedie force-pushed the fix-biff5 branch 4 times, most recently from 5afca64 to 2b1c0e4 Compare March 3, 2026 05:59
@alexander-beedie
Copy link
Contributor Author

(Updated with a couple of -very- minor comment/docstring clarifications)

@jmcnamara
Copy link
Collaborator

This patchset has a conflict due to another patch that got merged before it. Could you resolve that and rebase when you get a chance.

@alexander-beedie
Copy link
Contributor Author

alexander-beedie commented Mar 6, 2026

This patchset has a conflict due to another patch that got merged before it. Could you resolve that and rebase when you get a chance.

No problem - done 👍

Also updated the test in the previous PR with the suggested tweaks while rebasing/resolving (adding the associated issue number and a brief explanation).

@jmcnamara jmcnamara merged commit 3b815c7 into tafia:master Mar 6, 2026
6 checks passed
@alexander-beedie alexander-beedie deleted the fix-biff5 branch March 6, 2026 16:36
jmcnamara pushed a commit that referenced this pull request Mar 7, 2026
Repository owner deleted a comment from alexander-beedie Mar 7, 2026
Repository owner deleted a comment from alexander-beedie Mar 7, 2026
@jmcnamara
Copy link
Collaborator

Merged. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panic in xls.rs when BIFF5 (.xls) file has named range referencing sheet index >= 9

2 participants