This looks to me like a somewhat big oversight - it silently corrupts files and no warning nor error happens until it is too late.
After a few hours of debugging I feel like CopyEntryDirect is simply not functional if, for instance, one of the first few files of the ZipFile got deleted in a prior operation.
Let's put it this way - in direct mode, source stream and destination stream are the same.
That means writing on the destination is the same as writing on the source. You want to make extra sure you don't write over what you're going to read next. But if any file is deleted, this premise is broken - as deletions are treated, and copies are treated first, upon trying to copy the file SharpZipLib will happily overwrite the very file it's reading with itself.
Let's take an example, for instance the one I just spent a while looking into. A zip file with a 44 byte file, and then a 80kB file.
I decide to delete the first one. It has an offset of zero, and the next file has an offset of 44, because it's right after.
BeginUpdate => Delete => CommitUpdate
Upon commitment, updates are sorted, and the copy will now happen before the delete.
So this is what the library does - it takes the first update it has to handle, it's a copy! From <offset 44> to <wherever we are>. Since we have not done anything yet, <wherever we are> is offset 0. So, CopyEntryDirect copies byte 44<=>80044 to 0<=>80000.
There is no alignment, so just writing the "header" of that file we're copying, which totals 75 bytes, already overwrites the file itself. This causes massive, unrecoverable wreckage onto any zip file it touches!
This could maybe fixed with some minimum alignment the size of a "maximum zip header", or maybe by accepting some level of fragmentation in files and not moving stuff onto itself, or maybe temporarily copying the stuff to memory before committing it to file (not like the Safe mode, which renews the stream completely, something I cannot do in my project)
Sorry for the very long issue but I felt like this was important.
Reproduction Code
No response
Steps to reproduce
Add a few files in a zip using ZipFile, start with a tiny one (~40 bytes), give a long name to the others, then delete the first one you added.
Describe the bug
https://github.com/icsharpcode/SharpZipLib/blame/master/src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs#L2861
This looks to me like a somewhat big oversight - it silently corrupts files and no warning nor error happens until it is too late. After a few hours of debugging I feel like
CopyEntryDirect
is simply not functional if, for instance, one of the first few files of the ZipFile got deleted in a prior operation.Let's put it this way - in direct mode, source stream and destination stream are the same. That means writing on the destination is the same as writing on the source. You want to make extra sure you don't write over what you're going to read next. But if any file is deleted, this premise is broken - as deletions are treated, and copies are treated first, upon trying to copy the file SharpZipLib will happily overwrite the very file it's reading with itself.
Let's take an example, for instance the one I just spent a while looking into. A zip file with a 44 byte file, and then a 80kB file. I decide to delete the first one. It has an offset of zero, and the next file has an offset of 44, because it's right after.
BeginUpdate => Delete => CommitUpdate
Upon commitment, updates are sorted, and the copy will now happen before the delete. So this is what the library does - it takes the first update it has to handle, it's a copy! From
<offset 44>
to<wherever we are>
. Since we have not done anything yet,<wherever we are>
is offset 0. So,CopyEntryDirect
copies byte44<=>80044
to0<=>80000
. There is no alignment, so just writing the "header" of that file we're copying, which totals 75 bytes, already overwrites the file itself. This causes massive, unrecoverable wreckage onto any zip file it touches!This could maybe fixed with some minimum alignment the size of a "maximum zip header", or maybe by accepting some level of fragmentation in files and not moving stuff onto itself, or maybe temporarily copying the stuff to memory before committing it to file (not like the Safe mode, which renews the stream completely, something I cannot do in my project)
Sorry for the very long issue but I felt like this was important.
Reproduction Code
No response
Steps to reproduce
Add a few files in a zip using ZipFile, start with a tiny one (~40 bytes), give a long name to the others, then delete the first one you added.
Expected behavior
Something else than complete archive corurption
Operating System
Windows
Framework Version
.NET Framework 4.x, Unity
Tags
ZIP
Additional context
No response