ZipArchiveEntry.ExtractToFile corrupt destination files on exception

[JCFruit]

Description

When calling ZipArchiveEntry.ExtractToFile extension method with overwrite set to true, if ZipArchiveEntry.Open results in exception, existing destination file content is erased and file weights 0 bytes.

Reproduction Steps

1. Create folder with some files
2. Add files to archive that ZipArchive can't process. For example, protect it with password
3. Try to extract entries from archive using ExtractToFileAsync(destPath, overwrite: true)
4. After exception happens, destinationFileName is 0 bytes

using System.IO.Compression;

const string archive = @"D:\Tmp\MalformedArchive.zip";
const string folder = @"D:\Tmp\TestFolder";

await using var zipArchive = new ZipArchive(File.OpenRead(archive), ZipArchiveMode.Read, leaveOpen: false);

foreach (var zipArchiveEntry in zipArchive.Entries)
{
    var entryPath = zipArchiveEntry.FullName;
    
    var filePath = Path.Combine(folder, entryPath);
    
    await zipArchiveEntry.ExtractToFileAsync(filePath, true);
}

Expected behavior

On Exception to open the archive, original file is left intact

Actual behavior

On Exception to open the archive, original file content is deleted

Regression?

I don't know

Known Workarounds

No response

Configuration

• First I encountered this bug in Technitium DNS (.Net 9) running in LXC container
• I reproduced bug on Windows 10 using .Net 10

Other information

It probably happens because of this.

First, it's creating FileStream with FileMode.Create, and then it calls ZipArchiveEntry.Open. So by the time exception happens, original file is already overwritten.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io-compression
See info in area-owners.md if you want to be subscribed.

[iremyux]

Triage: While we cannot prevent getting into corrupted state during extraction, this specific case seems like something we can improve.

[JCFruit]

From my profane point of view fix seems straightforward - change order of operations from open file then open zip archive entry to open zip archive entry and then open file in sync and async versions of the method.

If I'm not missing something, should I just make this changes myself and open a PR?

[JuergenAuer]

change order of operations

Option: Do this complete.

Extract the complete archive into temporary files, so the original files are untouched.

await zipArchiveEntry.ExtractToFileAsync(filePath + ".temp", true);

If that is done completely, last step: Move the temp-files into the destination files.

So only the last move-operation "destroys" the old files.

But that may double the time to extract the archive.

May be a different, "very save" - method, if someone has a lot of corrputed zip archives.