I may have found a way to break ZipOutputStream...

Feb 7, 2010 at 5:00 AM
Edited Feb 8, 2010 at 3:24 AM

I created a ZipOutputStream with a MemoryStream as its underlying store (for the purpose of signing the zipfile when it's completed). I did some tests with just adding stuff to the file and then writing it to disk, and I'm seeing corrupted headers. Either I'm doing this horribly wrong or I have found a breaking use case.

Here's a bit of sample code that demonstrates the problem:

MemoryStream memStream = new MemoryStream();
mZipData = new ZipOutputStream(memStream);

mZipData.PutNextEntry("foo.txt");
byte[] buffer = System.Text.Encoding.ASCII.GetBytes("This is a file called foo.txt");
mZipData.Write(buffer, 0, buffer.Length);

mZipData.PutNextEntry(@"subdir/");

mZipData.PutNextEntry(@"subdir/bar.txt");
buffer = System.Text.Encoding.ASCII.GetBytes("This is another file, called bar.txt, " +
    "which lives in the subdirectory 'subdir'");
mZipData.Write(buffer, 0, buffer.Length);

FileStream outStream = new FileStream(@".\test.zip", FileMode.Create, FileAccess.Write);
memStream.Seek(0, 0);
memStream.WriteTo(outStream);

I think what might be happening is that under normal circumstances, the ZipOutputStream gets Disposed when it goes out of scope. This calls _FinishCurrentEntry, which on cursory examination looks like it then flushes the header and the entry contents. Since I'm directly accessing the underlying stream before anything has triggered _FinishCurrentEntry, the end of the archive is left in an unclean state. The corrupted result appears to only have the foo.txt entry. It's also possible I'm not doing the right thing with the subdirectory there...

Tim Keating

 

Coordinator
Feb 7, 2010 at 9:52 PM

Hi Tim,

you're doing the correct thing with the directory. 

The problem is, yes, the ZipOutputStream is not being Disposed.  Your output stream is not a complete zip file, until you Dispose the object. 

You use the words "under normal circumstances, the ZipOutputStream gets Disposed when it goes out of scope".  This is not correct usage, and your expectation is also not correct.  The Dispose() is not optional.  You must Dispose the object in order for it to work properly, as documented.    There's no way around that.   If you let the object drift out of scope, there is no guarantee that it will work as you want it to.  Call Dispose().

If there is any example in the DotNetZip documentation that does not show correct usage of the Dispose() method with ZipOutputStream, or the using clause, which is equivalent, then please let me know.  If there is any statement in the doc that seems to indicate that Dispose() is not necessary, let me know.

Here's the example of using ZipOutputStream with a memory stream:

   using (MemoryStream ms = new MemoryStream())
    {
        using (var output= new ZipOutputStream(ms))
        {
            output.Password = "VerySecret!";
            output.Encryption = EncryptionAlgorithm.WinZipAes256;
            output.PutNextEntry("entry1.txt");
            byte[] buffer= System.Text.Encoding.ASCII.GetBytes("This is the content for entry #1.");
            output.Write(buffer,0,buffer.Length);
            output.PutNextEntry("entry2.txt");  // this will be zero length
            output.PutNextEntry("entry3.txt");
            buffer= System.Text.Encoding.ASCII.GetBytes("This is the content for entry #3.");
            output.Write(buffer,0,buffer.Length);
        }

        // ms now contains the complete bytes for a zip file
    }

Feb 8, 2010 at 3:18 AM

Aha! I saw the FileStream example under the PutNextEntry documentation, which I presume this is based on. However, my previous .Net experience goes back to 2.0, and I completely missed the significance of the using blocks.

It might be worth explicitly calling out the need to Dispose the ZipOutputStream object in the overview. Short of that, though, this one's all on me.

TK

Feb 8, 2010 at 3:19 AM
Edited Feb 8, 2010 at 3:23 AM

Hmm, I actually figured this out not too long after posting this last night, and replied with a follow-up. I wonder where the heck that post went?

In any case, for posterity, here is the code I used to replace the last three lines, which made it work:

mZipData.Close();
FileStream outStream = new FileStream(@".\test.zip", FileMode.Create, FileAccess.Write);
BinaryWriter writer = new BinaryWriter(outStream);
writer.Write(mMemStream.GetBuffer());

Coordinator
Feb 9, 2010 at 6:36 PM

Ok, thanks for the tip; I will update the documentation to make this more explicit.