starred/sharpcompress
1
0
Fork 0
mirror of https://github.com/adamhathcock/sharpcompress.git synced 2026-02-08 21:21:57 +00:00
Code Issues 975 Packages Projects Releases 20 Wiki Activity

[PR #1054] [MERGED] Document ZipReader DirectoryEntry behavior and add verification test #1478

New Issue
Open
opened 2026-01-29 22:20:46 +00:00 by claunia · 0 comments
claunia commented 2026-01-29 22:20:46 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/adamhathcock/sharpcompress/pull/1054
Author: @Copilot
Created: 11/29/2025
Status: ✅ Merged
Merged: 11/29/2025
Merged by: @adamhathcock

Base: master ← Head: copilot/fix-zipreader-directory-entry

📝 Commits (6)

  • 0d9d82d Initial plan
  • f4dddce Changes before error encountered
  • a887390 Add multi-volume ZIP documentation to FORMATS.md
  • 7f911c5 Add documentation about ZipReader directory and central directory handling
  • 840e58f Update tests/SharpCompress.Test/Zip/ZipReaderTests.cs
  • c5da416 Apply code review feedback: improve documentation structure and add explicit DirectoryEntry case

📊 Changes

3 files changed (+53 additions, -1 deletions)

View changed files

📝 FORMATS.md (+6 -1)
📝 src/SharpCompress/Readers/Zip/ZipReader.cs (+8 -0)
📝 tests/SharpCompress.Test/Zip/ZipReaderTests.cs (+39 -0)

📄 Description

  • Investigate the ZipReader vs ZipArchive behavior for DirectoryEntry headers
  • Run tests to verify current behavior
  • Analyze whether ignoring DirectoryEntry is intentional
  • Determine if any fix is needed based on analysis
  • Add tests to document expected behavior
  • Add documentation to FORMATS.md about multi-volume ZIP requirement
  • Add documentation about ZipReader directory and central directory handling
  • Apply code review feedback
  • Run code review

Investigation Summary

The issue reports that ZipReader ignores DirectoryEntry headers completely. After thorough investigation:

Current Behavior Analysis:

  1. ZipReader (streaming): Processes LocalEntry headers with file data, skips DirectoryEntry headers from central directory
  2. ZipArchive (seekable): Uses DirectoryEntry headers from central directory and seeks to local entries for data

Key Findings:

  • For all tested single-volume ZIP files, both ZipReader and ZipArchive return identical entries
  • Directories are correctly identified from LocalEntry headers (where name ends with /)
  • In streaming mode, DirectoryEntry headers are redundant because:
    • All entries have corresponding LocalEntry headers in well-formed ZIPs
    • File data follows LocalEntry headers immediately
    • We cannot seek back in streaming mode anyway

Multi-Volume ZIP Limitation:

  • For multi-volume ZIPs, ZipReader returns 0 entries because file data is in other volume files
  • This is a fundamental limitation of forward-only streaming - cannot seek to other files
  • Users should use ZipArchive for multi-volume ZIPs (requires seekable streams)

Conclusion:

The current behavior is intentional and correct for streaming mode. The maintainer's comment "I think I'm intentionally skipping them" is confirmed correct.

Changes Made

  1. Added explicit DirectoryEntry case in ZipReader.GetEntries() with documentation explaining why it's intentionally skipped
  2. Added test ZipReader_Returns_Same_Entries_As_ZipArchive verifying both APIs return identical entries for single-volume ZIPs
  3. Updated FORMATS.md with new "Zip Format Notes" section documenting:
    • Multi-volume/split ZIP archives require ZipArchive
    • ZipReader processes entries from LocalEntry headers (including directories)
    • ZipReader intentionally skips DirectoryEntry headers from central directory as they are redundant
Original prompt

This section details on the original issue you should resolve

<issue_title>ZipReader ignored DirectoryEntry completely</issue_title>
<issue_description>ZipReader -> ignored DirectoryEntry completely | Could not extract the zip using ZipReader
095c871174/src/SharpCompress/Readers/Zip/ZipReader.cs (L63-L85)

ZipArchive -> processed DirectoryEntry | Could extract the zip using ZipArchive
095c871174/src/SharpCompress/Archives/Zip/ZipArchive.cs (L261-L296)</issue_description>

Comments on the Issue (you are @copilot in this section)

@adamhathcock I think I'm intentionally skipping them or used to. Will get copilot to check

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/adamhathcock/sharpcompress/pull/1054 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 11/29/2025 **Status:** ✅ Merged **Merged:** 11/29/2025 **Merged by:** [@adamhathcock](https://github.com/adamhathcock) **Base:** `master` ← **Head:** `copilot/fix-zipreader-directory-entry` --- ### 📝 Commits (6) - [`0d9d82d`](https://github.com/adamhathcock/sharpcompress/commit/0d9d82d7e6b125f4e8c7dba3c329c9fdcc09b9f4) Initial plan - [`f4dddce`](https://github.com/adamhathcock/sharpcompress/commit/f4dddcec8ec4fa3e6e23e6499dbf41e70a9c87c8) Changes before error encountered - [`a887390`](https://github.com/adamhathcock/sharpcompress/commit/a887390c23ccf03d197232d331830f2b6c6533d3) Add multi-volume ZIP documentation to FORMATS.md - [`7f911c5`](https://github.com/adamhathcock/sharpcompress/commit/7f911c5219b8c3365f63e1478d2c5a5fe2503a39) Add documentation about ZipReader directory and central directory handling - [`840e58f`](https://github.com/adamhathcock/sharpcompress/commit/840e58fc03ce14ac644cd253a276846c0f0eac73) Update tests/SharpCompress.Test/Zip/ZipReaderTests.cs - [`c5da416`](https://github.com/adamhathcock/sharpcompress/commit/c5da4167648248b053428ae0d24ce39f600f0430) Apply code review feedback: improve documentation structure and add explicit DirectoryEntry case ### 📊 Changes **3 files changed** (+53 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `FORMATS.md` (+6 -1) 📝 `src/SharpCompress/Readers/Zip/ZipReader.cs` (+8 -0) 📝 `tests/SharpCompress.Test/Zip/ZipReaderTests.cs` (+39 -0) </details> ### 📄 Description - [x] Investigate the ZipReader vs ZipArchive behavior for DirectoryEntry headers - [x] Run tests to verify current behavior - [x] Analyze whether ignoring DirectoryEntry is intentional - [x] Determine if any fix is needed based on analysis - [x] Add tests to document expected behavior - [x] Add documentation to FORMATS.md about multi-volume ZIP requirement - [x] Add documentation about ZipReader directory and central directory handling - [x] Apply code review feedback - [x] Run code review ## Investigation Summary The issue reports that `ZipReader` ignores `DirectoryEntry` headers completely. After thorough investigation: ### Current Behavior Analysis: 1. **ZipReader** (streaming): Processes `LocalEntry` headers with file data, skips `DirectoryEntry` headers from central directory 2. **ZipArchive** (seekable): Uses `DirectoryEntry` headers from central directory and seeks to local entries for data ### Key Findings: - For all tested single-volume ZIP files, both `ZipReader` and `ZipArchive` return **identical entries** - Directories are correctly identified from `LocalEntry` headers (where name ends with `/`) - In streaming mode, `DirectoryEntry` headers are redundant because: - All entries have corresponding `LocalEntry` headers in well-formed ZIPs - File data follows `LocalEntry` headers immediately - We cannot seek back in streaming mode anyway ### Multi-Volume ZIP Limitation: - For multi-volume ZIPs, `ZipReader` returns 0 entries because file data is in other volume files - This is a fundamental limitation of forward-only streaming - cannot seek to other files - Users should use `ZipArchive` for multi-volume ZIPs (requires seekable streams) ### Conclusion: The current behavior is **intentional and correct** for streaming mode. The maintainer's comment "I think I'm intentionally skipping them" is confirmed correct. ## Changes Made 1. Added explicit `DirectoryEntry` case in `ZipReader.GetEntries()` with documentation explaining why it's intentionally skipped 2. Added test `ZipReader_Returns_Same_Entries_As_ZipArchive` verifying both APIs return identical entries for single-volume ZIPs 3. Updated FORMATS.md with new "Zip Format Notes" section documenting: - Multi-volume/split ZIP archives require ZipArchive - ZipReader processes entries from LocalEntry headers (including directories) - ZipReader intentionally skips DirectoryEntry headers from central directory as they are redundant <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>ZipReader ignored DirectoryEntry completely</issue_title> > <issue_description>ZipReader -> ignored DirectoryEntry completely | Could not extract the zip using ZipReader > https://github.com/adamhathcock/sharpcompress/blob/095c871174585c436541d8dcb3b189e89040c093/src/SharpCompress/Readers/Zip/ZipReader.cs#L63-L85 > > ZipArchive -> processed DirectoryEntry | Could extract the zip using ZipArchive > https://github.com/adamhathcock/sharpcompress/blob/095c871174585c436541d8dcb3b189e89040c093/src/SharpCompress/Archives/Zip/ZipArchive.cs#L261-L296</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@adamhathcock</author><body> > I think I'm intentionally skipping them or used to. Will get copilot to check</body></comment_new> > </comments> > </details> - Fixes adamhathcock/sharpcompress#1046 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/adamhathcock/sharpcompress/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-29 22:20:46 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
duplicate
enhancement
feedback requested
not bug
pull-request
Mirrored from GitHub Pull Request
 question
up for grabs
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/sharpcompress#1478
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?