Bug 36560 - java.util.zip: Error parsing zip file with larger files in it
Summary: java.util.zip: Error parsing zip file with larger files in it
Status: RESOLVED FIXED
Alias: None
Product: classpath
Classification: Unclassified
Component: classpath (show other bugs)
Version: 0.97
: P3 major
Target Milestone: 0.99
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 41513 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-06-18 00:22 UTC by Daniel Noll
Modified: 2014-02-16 13:15 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-06-19 06:58:35


Attachments
gnu-zip-large-file-bug.zip (648 bytes, application/octet-stream)
2008-06-18 00:23 UTC, Daniel Noll
Details
proposed fix (391 bytes, patch)
2008-06-19 07:03 UTC, Jeroen Frijters
Details | Diff
Sample file which is broken by the patch (1008 bytes, application/octet-stream)
2008-06-30 04:06 UTC, Daniel Noll
Details
Detect encryption.patch (886 bytes, patch)
2008-06-30 04:43 UTC, Daniel Noll
Details | Diff
InflaterHuffmanTree.java.patch - better version (515 bytes, patch)
2009-10-01 02:24 UTC, Daniel Noll
Details | Diff
Actually corrupt zip file (145 bytes, application/octet-stream)
2009-10-01 02:26 UTC, Daniel Noll
Details

Note You need to log in before you can comment on or make changes to this bug.
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 Comment hidden (spam)