Update of zip file with DataDescriptor using ZipArchive results in corrupted file

[slakul]

When a zip archive is created some files can have DataDescriptor after LocalFileHeader and compressed data. It is indicated by the flag ZipArchiveEntry.BitFlagValues.DataDescriptor.

The update method in ZipArchive does not clear DataDescriptor flag and also does not write DataDescriptor after compressed data.

In Windows 10 the archive can be opened and files can be extracted, but when I try to add any file then the system reports that the archive is broken.

Steps to reproduce:

1. create zip file using ZipArchive (ZipArchiveMode.Create) and stream that cannot Seek (in order to use DataDescriptor)
2. add at least two not empty files with default compression
3. save the archive
4. open the created zip file using ZipArchive (ZipArchiveMode.Update)
5. modify one file in the archive
6. save the archive
7. open the archive in Windows 10 using File Explorer
8. add a file to the archive (use drag & drop)
9. Windows reports that the archive is corrupted

[carlossanlop]

@joshfree @karelz This is not a regression in 3.0, I was able to repro it in .NET Core 2.1. I'm going to chat offline with @JeremyKuhne because when attempting to repro in .NET Framework, the behavior was different.

Is this still a bug we need for 3.0 or should we change it to Future?

[carlossanlop]

@slakul thank you for reporting the bug. What kind of stream did you use that cannot seek (step 1 in your repro steps)? I had to create my own class that inherits from FileStream and forced the CanSeek property to always return false, but I suspect you used one of the stream classes we have that cannot be seeked (maybe PipeStream?).

[slakul]

@carlossanlop I was using files from MS Word (.docx), Ms Excel (.xlsx) and OpenOffice (.odt) and treated them as zip files. After analysis of the binary structure of the files I discovered how to reproduce the bug.

[carlossanlop]

Thank you @slakul.

Step 4 of your repro steps is the key here. If I do steps 1-3 in C#, then do steps 4-9 in 7-Zip or Windows File Explorer, the file does not get corrupted.

I may have a fix for this. I was able to make the bug go away and the existing unit tests are still passing. I need to add some unit tests, but first I need to understand the edge cases.

[carlossanlop]

The update method in ZipArchive does not clear DataDescriptor flag and also does not write DataDescriptor after compressed data.

@slakul, would you mind clarifying the second part of the sentence in case I am not understanding it correctly?

I have a fix for the first part of the sentence: You were right, we are missing code that would clear the DataDescriptor flag if we are in Update mode (step 4). We only turn on the DataDescriptor bit if we are in Create mode and CanSeek is false (step 1).

But I don't think the second part of that sentence is accurate: To update an existing zip file (step 4) requires creating a ZipArchive object inside a using directive, because the ZipArchive class is disposable. When the object is disposed, we call the code that writes the updates to the zip file, and one of the things that we always write is the GeneralPurposeBitFlag.

[slakul]

@carlossanlop The second part of the sentence was crucial in case DataDescriptor flag was not cleared. The fix for the first part of the sentence is sufficient to solve the issue.