Bug 36560

Summary: java.util.zip: Error parsing zip file with larger files in it
Product: classpath Reporter: Daniel Noll <daniel>
Component: classpathAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: major CC: bug-classpath, gnu_andrew, jackie.rosen
Priority: P3    
Version: 0.97   
Target Milestone: 0.99   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2008-06-19 06:58:35
Attachments: gnu-zip-large-file-bug.zip
proposed fix
Sample file which is broken by the patch
Detect encryption.patch
InflaterHuffmanTree.java.patch - better version
Actually corrupt zip file

Description Daniel Noll 2008-06-18 00:22:26 UTC
I will attach a sample zip file which contains an empty file slight under 50MB.

Sample code:

import java.io.File;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

import junit.framework.TestCase;
import org.apache.commons.io.IOUtils;

public class TestZipBug extends TestCase {
    public void testZipBug() throws Exception {
        ZipFile zip = new ZipFile(new File("D:\\gnu-zip-large-file-bug.zip"));
        ZipEntry entry = (ZipEntry) zip.entries().nextElement();
        IOUtils.toByteArray(zip.getInputStream(entry));
        // Omitting code to verify contents.
    }
}

Resulting error:
Caused by: gnu.java.util.zip.ZipException: Code lengths don't add up properly.
                at gnu.java.util.zip.InflaterInputStream.read(Unknown Source)
                at java.io.FilterInputStream.read(FilterInputStream.java:90)
Comment 1 Daniel Noll 2008-06-18 00:23:00 UTC
Created attachment 15777 [details]
gnu-zip-large-file-bug.zip

Attaching test file.
Comment 2 Jeroen Frijters 2008-06-19 07:01:17 UTC
It appears that the code != 65536 check in InflaterHuffmanTree.buildTree() is unnecessary (and causing this problem). I looked at the SharpZipLib version of this file (which is a port of this code to .NET) and they commented out the check (with a comment saying it is incompatible with dynamic trees and pkzip 2.04g).

I've confirmed that removing the check fixes this bug and produces the correct output.
Comment 3 Jeroen Frijters 2008-06-19 07:03:31 UTC
Created attachment 15786 [details]
proposed fix
Comment 4 Daniel Noll 2008-06-20 01:37:28 UTC
I can confirm that this patch fixes the issue both for the test file I submitted here and for the original file in which I noticed the problem.
Comment 5 Daniel Noll 2008-06-30 02:24:38 UTC
I think I've found a zip file where this check was necessary to stop it causing problems.

I'll see if I can find some data I'm allowed to attach here.
Comment 6 Daniel Noll 2008-06-30 04:06:13 UTC
Created attachment 15832 [details]
Sample file which is broken by the patch

Attached file which caused problems when the lines were commented out.
Comment 7 Daniel Noll 2008-06-30 04:31:50 UTC
It seems the problems are caused by the fact that the entry is encrypted yet Classpath's implementation ignores this and attempts to decompress it anyway.

It seems there is a 2-byte "general purpose flags" value stored before the compression method, and if bit 0 is set the entry is encrypted.  This could potentially be used to make getInputStream fail earlier to avoid that issue, in which case the existing patch for the large file itself can remain without causing a problem.
Comment 8 Daniel Noll 2008-06-30 04:43:49 UTC
Created attachment 15833 [details]
Detect encryption.patch

Proposed fix to properly detect encrypted entries and reject getInputStream() before trying to do anything which won't work.

Throws ZipException.  Not sure if it should have been this or another kind of IOException.
Comment 9 Daniel Noll 2009-10-01 01:23:53 UTC
*** Bug 41513 has been marked as a duplicate of this bug. ***
Comment 10 Daniel Noll 2009-10-01 02:24:54 UTC
Created attachment 18682 [details]
InflaterHuffmanTree.java.patch - better version

Still perform the check but allow for a special condition where there is only one code.
Comment 11 Daniel Noll 2009-10-01 02:26:20 UTC
Created attachment 18683 [details]
Actually corrupt zip file

Corrupt zip file sample I manually constructed to trigger the exception in the check.
Comment 12 Jeroen Frijters 2010-07-13 12:52:02 UTC
I've confirmed the patch by Daniel Noll as correct and committed a variation of his patch (with a modified exception message).
Comment 13 Andrew John Hughes 2012-02-15 23:44:42 UTC
Set to 0.99
Comment 14 Jackie Rosen 2014-02-16 13:15:33 UTC
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.