Skip to content

Fix XLSX loading with 64-bit ZIP data descriptors - #32

Merged
Shane32 merged 1 commit into
masterfrom
fix/zip64-data-descriptor-loading
Oct 19, 2026
Merged

Shane32 merged 1 commit into
masterfrom
fix/zip64-data-descriptor-loading

Conversation

@Shane32

@Shane32 Shane32 commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Loading an XLSX whose entries use 64-bit ZIP data descriptors without local ZIP64 size markers can throw BadReadException because the streaming reader advances to the wrong offset. Read package entries through the bundled ZipFile.Read central-directory reader while preserving CRC checks, temporary-stream disposal, and InvalidDataException for invalid packages.

Adds a 1,764-byte synthetic workbook containing only Sheet1!A1 = "test", with regression tests for reading and saving/reopening it. Both tests reproduce the failure before the fix, so no external workbook data is needed.

Validation:

  • Full test suite passed on .NET Core 2.1, .NET Core 3.1, .NET 6, .NET 8, .NET Framework 4.6.2, 4.8, and 4.8.1: 8,194 passed, 1,519 skipped, zero failures.
  • Debug solution build passed across all configured targets.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed compatibility with workbooks that use ZIP64 data descriptors, allowing them to be opened and saved reliably.
    • Loading a workbook now leaves the supplied input stream readable.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2e3bbe31-b13e-4134-92f6-a501338e8688
📥 Commits

Reviewing files that changed from the base of the PR and between 686684b and a9bbab6.

⛔ Files ignored due to path filters: 1
  • EPPlusTest/Workbooks/test-zip64-descriptor.xlsx is excluded by !**/*.xlsx
📒 Files selected for processing: 3
  • EPPlus/Packaging/ZipPackage.cs
  • EPPlusTest/EPPlusTest.Core.csproj
  • EPPlusTest/ExcelPackageTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

ZIP package loading now uses ZipFile.Read and extracts archive entries. Tests cover opening and reloading a ZIP64 data-descriptor workbook.

Changes: ZIP64 Archive Reading

Layer / File(s) Summary
Archive entry loading
EPPlus/Packaging/ZipPackage.cs
ZipPackage(Stream) reads entries through ZipFile.Read and extracts nonempty entries. It wraps ZIP parsing exceptions as InvalidDataException.
ZIP64 workbook validation
EPPlusTest/EPPlusTest.Core.csproj, EPPlusTest/ExcelPackageTests.cs
The test project copies the ZIP64 workbook fixture. Tests check worksheet and cell data after opening and reloading, and verify that the input stream remains readable.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ZipPackage
  participant ZipFile
  participant ZipEntry
  participant MemoryStream
  ZipPackage->>ZipFile: Read archive stream
  ZipFile-->>ZipPackage: Return archive entries
  ZipPackage->>ZipEntry: Extract each nonempty entry
  ZipEntry->>MemoryStream: Write extracted contents
Loading

Suggested reviewers: jankallman

Merge Risk: ⚪ Minimal · up to a9bba

The change switches package loading to the central-directory ZIP reader to fix XLSX files with 64-bit data descriptors, and adds regression tests for opening and saving such a workbook. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: fixing XLSX loading for 64-bit ZIP data descriptors.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Review found no blocking defects, with fixture validation and regression coverage supporting the targeted reader change.

No open issues 🎉

What changed in this PR

Fixes XLSX loading for entries with 64-bit ZIP data descriptors by using the bundled central-directory reader.

Changes:

  • Replaces streaming ZIP reads while preserving CRC validation, temporary-stream disposal, and invalid-package handling.
  • Adds loading and save/reopen regression tests.
  • Configures the synthetic workbook fixture to copy to test output.
File Description
EPPlusTest/​ExcelPackageTests.cs Adds ZIP64 descriptor regression tests and an input-stream ownership assertion.
EPPlusTest/​EPPlusTest.Core.csproj Copies the workbook fixture to test output.
EPPlus/​Packaging/​ZipPackage.cs Uses central-directory ZIP reading and entry extraction.

Review effort: Balanced

@Shane32
Shane32 merged commit 09a2110 into master Oct 19, 2026
5 checks passed
@Shane32
Shane32 deleted the fix/zip64-data-descriptor-loading branch October 19, 2026 15:24
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.

2 participants