icsharpcode
experiment with removing descriptors from zip file entries during updates
a spin off from #475 which makes the other change suggsted there to remove descriptors from entries during updates, in cases where the header is rewritten rather than just copied directly.
Put in a seperate PR to keep the experiment seperate in case the existing change is the one that's used.
Also adds an extra test, using a zip file created by a version of ZipOutputStream tweaked to not write the signature bytes, on top of the ones created directly during the tests.
The extra tests don't call TestArchive because it thinks that entries whose descriptors don't have signature bytes are invalid, which doesn't seem correct either?
I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.
Something i'm not sure about when looking at the existing code - what's the intention of the bit at https://github.com/icsharpcode/SharpZipLib/blob/a3bed391da11863277a597f6ae9ce2a9b8c20f83/src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs#L2082, where it enables descriptors for entries which are being encrypted - i'm not sure how that would effect the need for a descriptor?
(possibly the tests should be run with both plain and encrypted entries in case there are any differences?))
Well, just before that it checks if the CRC is present. If it isn't then it would have to be added in the descriptor. It might be the case that this couldn't be established before the crypto logic in some cases? It's a perfect example of where a comment would be really helpful.
Well, just before that it checks if the CRC is present. If it isn't then it would have to be added in the descriptor.
Would'nt that only be the case if the header can't be patched later?
In any case, any thoughts on the changed update.Command == UpdateCommand.Copy case in WriteLocalEntryHeader ?
@Numpsy I can't say that I fully understand the zipcrypto, but AFAIK it relates to how the CRC works when using encryption. The current implementation of OutputZipStream forces Descriptors on encrypted Store entries, even with a seekable stream. I have noticed this behaviour, but not investigated much further.
As for the copy, that does seem like what you would want, but the archive updating code is really hard to test correctly!
Required a bit more thought then I think, in case there are any other uninteded consequences.
(If the logic is related to ZipCrypto, that might mean there is something to think about for AES mode as well?)
I've been hoping to get back to this one, but haven't had any time recently to have a real think about it, but still hope to get back to it later.