Skip to content

Fix ZipArchive Update mode rewriting unchanged archives on Dispose #123719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alinpahontu2912 wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix ZipArchive Update mode rewriting unchanged archives on Dispose #123719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cc651fd
Fix ZipArchive Update mode rewriting unchanged archives on Dispose
alinpahontu2912 Jan 28, 2026
f45e40a
address comments
alinpahontu2912 Jan 29, 2026
5585426
solve brackets styling
alinpahontu2912 Jan 29, 2026
f960662
update failing test
alinpahontu2912 Jan 30, 2026
42563ac
address comments
alinpahontu2912 Feb 2, 2026
d091289
delete unnecessary unit test
alinpahontu2912 Feb 2, 2026
98dfe4e
address copilot feedback
alinpahontu2912 Feb 3, 2026
cf78e6a
remove whitespaces
alinpahontu2912 Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.Async.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,24 @@ protected virtual async ValueTask DisposeAsyncCore()
case ZipArchiveMode.Read:
break;
case ZipArchiveMode.Create:
await WriteFileAsync().ConfigureAwait(false);
break;
case ZipArchiveMode.Update:
default:
Debug.Assert(_mode == ZipArchiveMode.Update || _mode == ZipArchiveMode.Create);
await WriteFileAsync().ConfigureAwait(false);
Debug.Assert(_mode == ZipArchiveMode.Update);
// Only write if the archive has been modified
if (IsModified)
{
await WriteFileAsync().ConfigureAwait(false);
}
else
{
// Even if we didn't write, unload any entry buffers that may have been loaded
foreach (ZipArchiveEntry entry in _entries)
{
await entry.UnloadStreamsAsync().ConfigureAwait(false);
}
}
break;
}
}
Expand Down
52 changes: 49 additions & 3 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,24 @@ protected virtual void Dispose(bool disposing)
case ZipArchiveMode.Read:
break;
case ZipArchiveMode.Create:
WriteFile();
break;
case ZipArchiveMode.Update:
default:
Debug.Assert(_mode == ZipArchiveMode.Update || _mode == ZipArchiveMode.Create);
WriteFile();
Debug.Assert(_mode == ZipArchiveMode.Update);
// Only write if the archive has been modified
if (IsModified)
{
WriteFile();
}
else
{
// Even if we didn't write, unload any entry buffers that may have been loaded
foreach (ZipArchiveEntry entry in _entries)
{
entry.UnloadStreams();
}
}
break;
}
}
Expand All @@ -311,7 +325,6 @@ protected virtual void Dispose(bool disposing)
}
}
}

/// <summary>
/// Finishes writing the archive and releases all resources used by the ZipArchive object, unless the object was constructed with leaveOpen as true. Any streams from opened entries in the ZipArchive still open will throw exceptions on subsequent writes, as the underlying streams will have been closed.
/// </summary>
Expand Down Expand Up @@ -379,6 +392,39 @@ private set
// New entries in the archive won't change its state.
internal ChangeState Changed { get; private set; }

/// <summary>
/// Determines whether the archive has been modified and needs to be written.
/// </summary>
private bool IsModified
{
get
{
// A new archive (created on empty stream) always needs to write the structure
if (_archiveStream.Length == 0)
{
return true;
}
// Archive-level changes (e.g., comment)
if (Changed != ChangeState.Unchanged)
{
return true;
}
// Any deleted entries
if (_firstDeletedEntryOffset != long.MaxValue)
{
return true;
}
// Check if any entry was modified or added
foreach (ZipArchiveEntry entry in _entries)
{
if (!entry.OriginallyInArchive || entry.Changes != ChangeState.Unchanged)
return true;
}

return false;
}
}

private ZipArchiveEntry DoCreateEntry(string entryName, CompressionLevel? compressionLevel)
{
ArgumentException.ThrowIfNullOrEmpty(entryName);
Expand Down
32 changes: 19 additions & 13 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,6 @@ private async Task<MemoryStream> GetUncompressedDataAsync(CancellationToken canc
}
}
}

// if they start modifying it and the compression method is not "store", we should make sure it will get deflated
if (CompressionMethod != ZipCompressionMethod.Stored)
{
CompressionMethod = ZipCompressionMethod.Deflate;
}
}

return _storedUncompressedData;
Expand Down Expand Up @@ -270,8 +264,6 @@ private async Task<WrappedStream> OpenInUpdateModeAsync(bool loadExistingContent
await ThrowIfNotOpenableAsync(needToUncompress: true, needToLoadIntoMemory: true, cancellationToken).ConfigureAwait(false);
}

_everOpenedForWrite = true;
Changes |= ZipArchive.ChangeState.StoredData;
_currentlyOpenForWrite = true;

if (loadExistingContent)
Expand All @@ -282,14 +274,15 @@ private async Task<WrappedStream> OpenInUpdateModeAsync(bool loadExistingContent
{
_storedUncompressedData?.Dispose();
_storedUncompressedData = new MemoryStream();
// Opening with loadExistingContent: false discards existing content, which is a modification
MarkAsModified();
}

_storedUncompressedData.Seek(0, SeekOrigin.Begin);

return new WrappedStream(_storedUncompressedData, this, thisRef =>
{
thisRef!._currentlyOpenForWrite = false;
});
return new WrappedStream(_storedUncompressedData, this,
onClosed: thisRef => thisRef!._currentlyOpenForWrite = false,
notifyEntryOnWrite: true);
}

private async Task<(bool, string?)> IsOpenableAsync(bool needToUncompress, bool needToLoadIntoMemory, CancellationToken cancellationToken)
Expand Down Expand Up @@ -351,6 +344,19 @@ private async Task<bool> WriteLocalFileHeaderAsync(bool isEmptyFile, bool forceW
private async Task WriteLocalFileHeaderAndDataIfNeededAsync(bool forceWrite, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

// Check if the entry's stored data was actually modified (StoredData flag is set).
// If _storedUncompressedData is loaded but StoredData is not set, it means the entry
// was opened for update but no writes occurred - we should use the original compressed bytes.
bool storedDataModified = (Changes & ZipArchive.ChangeState.StoredData) != 0;

// If _storedUncompressedData is loaded but not modified, clear it so we use _compressedBytes
if (_storedUncompressedData != null && !storedDataModified)
{
await _storedUncompressedData.DisposeAsync().ConfigureAwait(false);
_storedUncompressedData = null;
}

// _storedUncompressedData gets frozen here, and is what gets written to the file
if (_storedUncompressedData != null || _compressedBytes != null)
{
Expand Down Expand Up @@ -466,7 +472,7 @@ private ValueTask WriteDataDescriptorAsync(CancellationToken cancellationToken)
return _archive.ArchiveStream.WriteAsync(dataDescriptor.AsMemory(0, bytesToWrite), cancellationToken);
}

private async Task UnloadStreamsAsync()
internal async Task UnloadStreamsAsync()
{
if (_storedUncompressedData != null)
{
Expand Down
50 changes: 38 additions & 12 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,16 +508,15 @@ private MemoryStream GetUncompressedData()
}
}

// if they start modifying it and the compression method is not "store", we should make sure it will get deflated
if (CompressionMethod != ZipCompressionMethod.Stored)
{
CompressionMethod = ZipCompressionMethod.Deflate;
}
// NOTE: CompressionMethod normalization is deferred to MarkAsModified() to avoid
// corrupting entries that are opened in Update mode but not actually written to.
// If we normalized here and the entry wasn't modified, we'd write a header with
// CompressionMethod=Deflate but the original _compressedBytes would still be in
// their original format (e.g., Deflate64), producing an invalid entry.
}

return _storedUncompressedData;
}

// does almost everything you need to do to forget about this entry
// writes the local header/data, gets rid of all the data,
// closes all of the streams except for the very outermost one that
Expand Down Expand Up @@ -882,8 +881,6 @@ private WrappedStream OpenInUpdateMode(bool loadExistingContent = true)
ThrowIfNotOpenable(needToUncompress: true, needToLoadIntoMemory: true);
}

_everOpenedForWrite = true;
Changes |= ZipArchive.ChangeState.StoredData;
_currentlyOpenForWrite = true;

if (loadExistingContent)
Expand All @@ -894,14 +891,31 @@ private WrappedStream OpenInUpdateMode(bool loadExistingContent = true)
{
_storedUncompressedData?.Dispose();
_storedUncompressedData = new MemoryStream();
// Opening with loadExistingContent: false discards existing content, which is a modification
MarkAsModified();
}

_storedUncompressedData.Seek(0, SeekOrigin.Begin);

return new WrappedStream(_storedUncompressedData, this, thisRef =>
return new WrappedStream(_storedUncompressedData, this,
onClosed: thisRef => thisRef!._currentlyOpenForWrite = false,
notifyEntryOnWrite: true);
}

/// <summary>
/// Marks this entry as modified, indicating that its data has changed and needs to be rewritten.
/// </summary>
internal void MarkAsModified()
{
_everOpenedForWrite = true;
Changes |= ZipArchive.ChangeState.StoredData;

// Normalize compression method when actually modifying - Deflate64 data will be
// re-compressed as Deflate since we don't support writing Deflate64.
if (CompressionMethod != ZipCompressionMethod.Stored)
{
thisRef!._currentlyOpenForWrite = false;
});
CompressionMethod = ZipCompressionMethod.Deflate;
}
}

private bool IsOpenable(bool needToUncompress, bool needToLoadIntoMemory, out string? message)
Expand Down Expand Up @@ -1171,6 +1185,18 @@ private bool WriteLocalFileHeader(bool isEmptyFile, bool forceWrite)

private void WriteLocalFileHeaderAndDataIfNeeded(bool forceWrite)
{
// Check if the entry's stored data was actually modified (StoredData flag is set).
// If _storedUncompressedData is loaded but StoredData is not set, it means the entry
// was opened for update but no writes occurred - we should use the original compressed bytes.
bool storedDataModified = (Changes & ZipArchive.ChangeState.StoredData) != 0;

// If _storedUncompressedData is loaded but not modified, clear it so we use _compressedBytes
if (_storedUncompressedData != null && !storedDataModified)
{
_storedUncompressedData.Dispose();
_storedUncompressedData = null;
}

// _storedUncompressedData gets frozen here, and is what gets written to the file
if (_storedUncompressedData != null || _compressedBytes != null)
{
Expand Down Expand Up @@ -1396,7 +1422,7 @@ private int PrepareToWriteDataDescriptor(Span<byte> dataDescriptor)
return bytesToWrite;
}

private void UnloadStreams()
internal void UnloadStreams()
{
_storedUncompressedData?.Dispose();
_compressedBytes = null;
Expand Down
27 changes: 23 additions & 4 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCustomStreams.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,28 @@ internal sealed class WrappedStream : Stream
// Delegate that will be invoked on stream disposing
private readonly Action<ZipArchiveEntry?>? _onClosed;

// When true, notifies the entry on first write operation
private bool _notifyEntryOnWrite;

// Instance that will be passed to _onClose delegate
private readonly ZipArchiveEntry? _zipArchiveEntry;
private bool _isDisposed;

internal WrappedStream(Stream baseStream, bool closeBaseStream)
: this(baseStream, closeBaseStream, null, null) { }
: this(baseStream, closeBaseStream, entry: null, onClosed: null, notifyEntryOnWrite: false) { }

private WrappedStream(Stream baseStream, bool closeBaseStream, ZipArchiveEntry? entry, Action<ZipArchiveEntry?>? onClosed)
private WrappedStream(Stream baseStream, bool closeBaseStream, ZipArchiveEntry? entry, Action<ZipArchiveEntry?>? onClosed, bool notifyEntryOnWrite)
{
_baseStream = baseStream;
_closeBaseStream = closeBaseStream;
_onClosed = onClosed;
_notifyEntryOnWrite = notifyEntryOnWrite;
_zipArchiveEntry = entry;
_isDisposed = false;
}

internal WrappedStream(Stream baseStream, ZipArchiveEntry entry, Action<ZipArchiveEntry?>? onClosed)
: this(baseStream, false, entry, onClosed) { }
internal WrappedStream(Stream baseStream, ZipArchiveEntry entry, Action<ZipArchiveEntry?>? onClosed, bool notifyEntryOnWrite = false)
: this(baseStream, false, entry, onClosed, notifyEntryOnWrite) { }

public override long Length
{
Expand Down Expand Up @@ -144,6 +148,7 @@ public override void SetLength(long value)
ThrowIfCantSeek();
ThrowIfCantWrite();

NotifyWrite();
_baseStream.SetLength(value);
}

Expand All @@ -152,6 +157,7 @@ public override void Write(byte[] buffer, int offset, int count)
ThrowIfDisposed();
ThrowIfCantWrite();

NotifyWrite();
_baseStream.Write(buffer, offset, count);
}

Expand All @@ -160,6 +166,7 @@ public override void Write(ReadOnlySpan<byte> source)
ThrowIfDisposed();
ThrowIfCantWrite();

NotifyWrite();
_baseStream.Write(source);
}

Expand All @@ -168,6 +175,7 @@ public override void WriteByte(byte value)
ThrowIfDisposed();
ThrowIfCantWrite();

NotifyWrite();
_baseStream.WriteByte(value);
}

Expand All @@ -176,6 +184,7 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati
ThrowIfDisposed();
ThrowIfCantWrite();

NotifyWrite();
return _baseStream.WriteAsync(buffer, offset, count, cancellationToken);
}

Expand All @@ -184,9 +193,19 @@ public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationTo
ThrowIfDisposed();
ThrowIfCantWrite();

NotifyWrite();
return _baseStream.WriteAsync(buffer, cancellationToken);
}

private void NotifyWrite()
{
if (_notifyEntryOnWrite)
{
_zipArchiveEntry?.MarkAsModified();
_notifyEntryOnWrite = false; // Only notify once
}
}

public override void Flush()
{
ThrowIfDisposed();
Expand Down
Loading
Loading