Fix ZipArchive Update removing data descriptors by bwinsley · Pull Request #126447 · dotnet/runtime

bwinsley · 2026-04-02T02:49:09Z

Fix ZipArchive Update mode corruption when entries have data descriptors (bit 3)

Description

When opening an existing ZIP in ZipArchiveMode.Update and disposing (which triggers the write-back), the offset calculations in WriteFileCalculateOffsets and the stream seeking in WriteLocalFileHeaderAndDataIfNeeded/WriteLocalFileHeaderAndDataIfNeededAsync did not account for the data descriptor bytes (12–24 bytes depending on format) that follow compressed data when general purpose bit flag bit 3 is set. This caused new or shifted entries to overwrite data descriptors, corrupting the archive.

This commonly affects archives created on non-seekable streams (which always set bit 3) or by external tools such as Java's ZipOutputStream, Azure blob storage SDKs, and similar.

This is a .NET 10 regression introduced by PR #102704's selective-rewrite optimization.

Changes

Offset calculation fix (`ZipArchive.cs`, `ZipArchive.Async.cs`)

ComputeEntryEndOffsets() — New method that precomputes EndOfLocalEntryData for each originally-in-archive entry in a single O(n) reverse pass. Since _entries is sorted by local header offset, each entry's end boundary is the next original entry's OffsetOfLocalHeader, or _centralDirectoryStart for the last entry. This naturally accounts for any trailing data (data descriptors, padding, etc.) without any stream I/O.
WriteFileCalculateOffsets — Now uses entry.EndOfLocalEntryData instead of GetOffsetOfCompressedData() + CompressedLength, which did not include data descriptor bytes.
Both WriteFile() and WriteFileAsync() call ComputeEntryEndOffsets() before processing entries.

Metadata-only seek fix (`ZipArchiveEntry.cs`, `ZipArchiveEntry.Async.cs`)

After writing the local file header for an unchanged/metadata-only entry, the stream now seeks to the absolute EndOfLocalEntryData position instead of using a relative seek by _compressedSize. This single seek correctly advances past both compressed data and any trailing data descriptor, with zero stream reads.

Bit 3 (data descriptor flag) handling

WriteLocalFileHeaderInitialize now clears bit 3 only when the header is actually being written (not when it's skipped for unchanged entries) and only for seekable/empty-file paths. This means unchanged entries that skip header writing preserve their original bit 3 flag, keeping the local header, central directory, and on-disk data descriptor consistent — no save/restore or seek-back patching needed.
For entries whose headers are rewritten (force-written after a rewrite point), bit 3 is correctly cleared since the sizes are written directly into the header. Any orphaned data descriptor bytes become harmless dead space — compliant readers ignore them when bit 3 is not set.

Testing

Three new regression tests in zip_UpdateTests.cs, all parameterized on bool async via [Theory]/[MemberData] to exercise sync Dispose and async DisposeAsync code paths:

`Update_DataDescriptorSignature_IsCorrectlyWrittenAndPreserved`

Creates a 3-entry archive via a non-seekable stream (forces bit 3 / data descriptors) with NoCompression, reopens in Update mode, adds a new entry. Performs a structural binary walk of the updated archive: EOCD → central directory → local file headers, verifying bit 3 flags on original entries and data descriptor signatures at the correct computed byte offsets.

`Update_DataDescriptorWithDeletedEntry_PreservesArchive`

Creates a 5-entry archive via a non-seekable stream, reopens in Update mode, deletes a middle entry. Verifies the remaining 4 entries are readable with correct data. Exercises offset recalculation in ComputeEntryEndOffsets when entries with data descriptors are removed and subsequent entries must shift.

`Update_DataDescriptorWithMetadataOnlyChange_PreservesArchive`

Creates a 3-entry archive via a non-seekable stream, reopens in Update mode, changes LastWriteTime on the middle entry without opening its stream (exercises the metadata-only rewrite path), adds a new entry. Reopens in Read mode and verifies all original entries' data intact, the metadata change was preserved, and the new entry is correct.

Test results

All 1739 tests pass with 0 errors, 0 failures (1733 existing + 6 new test runs).

Copilot

Pull request overview

Adds a regression test in System.IO.Compression to validate ZIP archives created in “streaming” mode (non-seekable stream, data-descriptor bit set) remain structurally valid after reopening in ZipArchiveMode.Update and adding an entry.

Changes:

Added System.Buffers.Binary usage to inspect ZIP bytes.
Added a new [Theory] test that creates a streaming ZIP, validates data-descriptor markers, updates the archive by adding an entry, then validates readability/structure again.

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs

…pressed data

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs

… in compressed data

…ditions into functions

bwinsley · 2026-04-02T06:44:53Z

@dotnet-policy-service agree company="Displayr"

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs

rzikm · 2026-04-07T10:58:18Z

For entries whose headers ARE rewritten (metadata-only changes like LastWriteTime), this would leave bit 3 = 0 in the on-disk local header while the data descriptor bytes remain physically present — creating orphaned bytes a sequential parser cannot navigate.

One could argue that entries with DataDescriptor bit aren't sequentially readable anyway, since the size of the entry is not known up-front (size in the local header is 0), you don't know where the DataDescriptor is (looking for the magic bytes header is not reliable).

I agree that we should preserve the bits and the data descriptor though.

rzikm · 2026-04-07T11:06:51Z

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs

+            ComputeEntryEndOffsets();
+
+            for (int i = 0; i < _entries.Count; i++)


can we avoid iterating over the _entries twice?

Unfortunately not without heavy refactoring as ComputeEntryEndOffsets() iterates backwards and this loop on L371 iterates forwards and needs the values from ComputeEntryEndOffsets(). This should still be O(n) anyways and ComputeEntryEndOffsets() is not a very heavy/intensive function

I mean, you can do something like

_entries[i].EndOfLocalEntryData = i < _entries.Length - 1 ? _entries[i + 1].OffsetOfLocalHeader : _centralDirectoryStart

It might actually make sense to do this at the time of actually reading the central directory for the first time, that way we don't need to care about OriginallyInTheArchive

Excellent pick up. I've moved it to where we read the central directory now

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs

…he source instead of just patching it

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs

…scriptor rewrites

… data descriptor rewrites" This reverts commit 1fc1762.

… rewrites in ZipArchive Update mode

…ary checks

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

bwinsley · 2026-04-08T04:56:02Z

@rzikm @alinpahontu2912 Have implemented suggestions where possible and left replies where not. Please let me know if anything is missing or needs updating!

…ctory

alinpahontu2912 · 2026-04-10T07:23:08Z

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs


            entriesToWrite = new(_entries.Count);
-            foreach (ZipArchiveEntry entry in _entries)
+


why change this ?

alinpahontu2912 · 2026-04-10T07:31:59Z

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs

                entriesToWrite = new(_entries.Count);

-                foreach (ZipArchiveEntry entry in _entries)
+                for (int i = 0; i < _entries.Count; i++)


same question about changing this foreach loop

alinpahontu2912 · 2026-04-10T07:33:13Z

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs


    // return value is true if we allocated an extra field for 64 bit headers, un/compressed size
-    private async Task<bool> WriteLocalFileHeaderAsync(bool isEmptyFile, bool forceWrite, CancellationToken cancellationToken)
+    private async Task<bool> WriteLocalFileHeaderAsync(bool isEmptyFile, bool forceWrite, CancellationToken cancellationToken, bool preserveDataDescriptor = false)


Usually cancellationtoken is the last parameter, what do we think about this @rzikm ?

alinpahontu2912 · 2026-04-10T07:34:25Z

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs

+            // directly into the header, so a data descriptor is not needed.
+            if (isEmptyFile || _archive.ArchiveStream.CanSeek)
+            {
+                if (preserveDataDescriptor && (_generalPurposeBitFlag & BitFlagValues.DataDescriptor) != 0)


(_generalPurposeBitFlag & BitFlagValues.DataDescriptor) != 0 won't this always be true if preserveDataDescriptor is true ?

Add a unit test

5c8f4a6

Copilot AI review requested due to automatic review settings April 2, 2026 02:49

github-actions bot added the area-System.IO.Compression label Apr 2, 2026

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 2, 2026

Copilot started reviewing on behalf of bwinsley April 2, 2026 02:50 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

bwinsley added 3 commits April 2, 2026 16:37

Clean up unit test

517b545

Add regression test for update - data descriptor handling

e3cbbec

fix nextFileOffset to account for data descriptors in update mode

ed81677

Copilot AI review requested due to automatic review settings April 2, 2026 05:54

Copilot started reviewing on behalf of bwinsley April 2, 2026 05:56 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

bwinsley added 2 commits April 2, 2026 17:05

fix metadata-only rewrite path to see past data descriptors after com…

b9f2213

…pressed data

clear data descriptor bit when rewriting an entry as empty

ffcfc41

Copilot AI review requested due to automatic review settings April 2, 2026 06:08

Copilot started reviewing on behalf of bwinsley April 2, 2026 06:09 View session

minor: clean up comments

9204e1c

Copilot AI reviewed Apr 2, 2026

View reviewed changes

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Outdated Show resolved Hide resolved

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Outdated Show resolved Hide resolved

bwinsley added 4 commits April 2, 2026 17:19

conduct test using no compression to defend against checking for 'PK'…

d294ed0

… in compressed data

Precalculate entry end offsets for performance + refactor new code ad…

312f586

…ditions into functions

preserve bit 3 in local header when data descriptor bytes remain

6b5ac4d

use captured zip64 sizes instead of AreSizesTooLarge

85212c1

bwinsley requested a review from Copilot April 2, 2026 06:42

Copilot started reviewing on behalf of bwinsley April 2, 2026 06:43 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Outdated Show resolved Hide resolved

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs Outdated Show resolved Hide resolved

revert AreSizesTooLarge

45c7a0b

bwinsley requested a review from Copilot April 2, 2026 06:53

Copilot started reviewing on behalf of bwinsley April 2, 2026 06:54 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Outdated Show resolved Hide resolved

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs Outdated Show resolved Hide resolved

alinpahontu2912 reviewed Apr 7, 2026

View reviewed changes

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs Show resolved Hide resolved

alinpahontu2912 reviewed Apr 7, 2026

View reviewed changes

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs Show resolved Hide resolved

rzikm reviewed Apr 7, 2026

View reviewed changes

bwinsley added 4 commits April 7, 2026 21:45

Add debug assertion

0031e21

Make unit test more light weight

5d53358

Add unit test to test deletion in update mode

cb3a0ba

Fix data descriptor signature being cleared when updating header at t…

0774808

…he source instead of just patching it

Copilot AI review requested due to automatic review settings April 7, 2026 13:26

Copilot started reviewing on behalf of bwinsley April 7, 2026 13:28 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs Show resolved Hide resolved

src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs Show resolved Hide resolved

bwinsley added 5 commits April 8, 2026 11:10

Preserve bit 3 and write zeros for CRC/sizes in metadata-only data de…

1fc1762

…scriptor rewrites

Revert "Preserve bit 3 and write zeros for CRC/sizes in metadata-only…

b1168c1

… data descriptor rewrites" This reverts commit 1fc1762.

Preserve data descriptor (bit 3) and set CRC32=0 during metadata-only…

61cc279

… rewrites in ZipArchive Update mode

Refactor tests by extracting logic to functions and removing unnecess…

e3d461a

…ary checks

Simplify logic of setting crc32

8524fe4

Copilot AI review requested due to automatic review settings April 8, 2026 04:18

Copilot started reviewing on behalf of bwinsley April 8, 2026 04:19 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

bwinsley requested review from alinpahontu2912 and rzikm April 8, 2026 04:55

build-analysis bot mentioned this pull request Apr 8, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

karelz assigned alinpahontu2912 Apr 9, 2026

Move EndOfLocalEntryData computation to when we read the central dire…

d4a7525

…ctory

alinpahontu2912 reviewed Apr 10, 2026

View reviewed changes

		ComputeEntryEndOffsets();

		for (int i = 0; i < _entries.Count; i++)


		entriesToWrite = new(_entries.Count);
		foreach (ZipArchiveEntry entry in _entries)

Conversation

bwinsley commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix ZipArchive Update mode corruption when entries have data descriptors (bit 3)

Description

Changes

Offset calculation fix (ZipArchive.cs, ZipArchive.Async.cs)

Metadata-only seek fix (ZipArchiveEntry.cs, ZipArchiveEntry.Async.cs)

Bit 3 (data descriptor flag) handling

Testing

Update_DataDescriptorSignature_IsCorrectlyWrittenAndPreserved

Update_DataDescriptorWithDeletedEntry_PreservesArchive

Update_DataDescriptorWithMetadataOnlyChange_PreservesArchive

Test results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

bwinsley commented Apr 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rzikm commented Apr 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bwinsley commented Apr 8, 2026

Uh oh!

bwinsley commented Apr 2, 2026 •

edited

Loading

Offset calculation fix (`ZipArchive.cs`, `ZipArchive.Async.cs`)

Metadata-only seek fix (`ZipArchiveEntry.cs`, `ZipArchiveEntry.Async.cs`)

`Update_DataDescriptorSignature_IsCorrectlyWrittenAndPreserved`

`Update_DataDescriptorWithDeletedEntry_PreservesArchive`

`Update_DataDescriptorWithMetadataOnlyChange_PreservesArchive`