Fix ZipArchive Update mode rewriting unchanged archives on Dispose

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

@@ -147,10 +147,16 @@ 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);
+                        }
                         break;
                 }
             }

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

@@ -297,10 +297,16 @@ 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();
+                            }
                             break;
                     }
                 }
@@ -379,6 +385,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);

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

@@ -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;
@@ -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)
@@ -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)
@@ -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)
         {

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

@@ -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
@@ -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)
@@ -894,14 +891,34 @@ 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>
+        /// <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)
@@ -1171,6 +1188,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)
             {

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

@@ -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
         {
@@ -144,6 +148,7 @@ public override void SetLength(long value)
             ThrowIfCantSeek();
             ThrowIfCantWrite();
 
+            NotifyWrite();
             _baseStream.SetLength(value);
         }
 
@@ -152,6 +157,7 @@ public override void Write(byte[] buffer, int offset, int count)
             ThrowIfDisposed();
             ThrowIfCantWrite();
 
+            NotifyWrite();
             _baseStream.Write(buffer, offset, count);
         }
 
@@ -160,6 +166,7 @@ public override void Write(ReadOnlySpan<byte> source)
             ThrowIfDisposed();
             ThrowIfCantWrite();
 
+            NotifyWrite();
             _baseStream.Write(source);
         }
 
@@ -168,6 +175,7 @@ public override void WriteByte(byte value)
             ThrowIfDisposed();
             ThrowIfCantWrite();
 
+            NotifyWrite();
             _baseStream.WriteByte(value);
         }
 
@@ -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);
         }
 
@@ -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();

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

@@ -824,8 +824,7 @@ public static IEnumerable<object[]> EmptyFiles()
         /// 2. EmptyFileCompressedWrongSize has
         /// Deflate 0x08, _uncompressedSize 0, _compressedSize 4, compressed data: 0xBAAD0300 (just bad data)
         /// ZipArchive is not expected to make any changes to the compression method of an archive entry unless
-        /// it's been changed. If it has been changed, ZipArchive is expected to change compression method to
-        /// Stored (0x00) and ignore "bad" compressed size
+        /// it's been modified. Opening an entry stream without writing does not trigger a rewrite.
         /// </summary>
         [Theory]
         [MemberData(nameof(EmptyFiles))]
@@ -852,30 +851,27 @@ public async Task ReadArchive_WithEmptyDeflatedFile(byte[] fileBytes, bool async
                 zip = await CreateZipArchive(async, testStream, ZipArchiveMode.Update, leaveOpen: true);
 
                 var zipEntryStream = await OpenEntryStream(async, zip.Entries[0]);
-                // dispose after opening an entry will rewrite the archive
+                // Opening and disposing an entry stream without writing does not mark the archive as modified,
+                // so no rewrite will occur
                 await DisposeStream(async, zipEntryStream);
 
                 await DisposeZipArchive(async, zip);
 
                 fileContent = testStream.ToArray();
 
-                // compression method should change to "uncompressed" (Stored = 0x0)
-                Assert.Equal(0, fileContent[8]);
+                // compression method should remain unchanged since we didn't write anything
+                Assert.Equal(firstEntryCompressionMethod, fileContent[8]);
 
-                // extract and check the file. should stay empty.
+                // extract and check the file
                 zip = await CreateZipArchive(async, testStream, ZipArchiveMode.Update);
 
                 ZipArchiveEntry entry = zip.GetEntry(ExpectedFileName);
+                // The entry retains its original properties since it was never modified
                 Assert.Equal(0, entry.Length);
-                Assert.Equal(0, entry.CompressedLength);
-                Stream entryStream = await OpenEntryStream(async, entry);
-                Assert.Equal(0, entryStream.Length);
-                await DisposeStream(async, entryStream);
 
                 await DisposeZipArchive(async, zip);
             }
         }
-
         /// <summary>
         /// Opens an empty file that has a 64KB EOCD comment.
         /// Adds two 64KB text entries. Verifies they can be read correctly.