Commons Compress
  1. Commons Compress
  2. COMPRESS-94

Creating zip files with many entries will ocassionally produce corrupted output

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.1
    • Component/s: None
    • Labels:
      None
    • Environment:

      Windows 2003 Server 64 bit, Java 6.0

      Description

      Our application produces large numbers of zip files, often with 1000's of similarly named files contained within the zip.
      When we switched from the standard JDK zip classes to those in commons compress, we would ocassionally produce a zip file that had corrupted index entries and would fail to unzip successfully using 7-zip, winzip, etc.

      Debugging the zip creation showed that the the wrong offsets were being returned from the hashmap in ZipOutputStream for the entries that were being corrupted. Further analysis revealed that this occurred when the filenames being added had a hash collision with another entry in the same output zip (which appears to happen quite frequently for us).

      The issue appears to stem from the fact that ZipArchiveEntry can store the entry name either in its superclass if passed in on the ctor or in its own member attribute if set later via setName(). Not sure whether this functionality is really required? Regardless, the root cause of the bug is that the equals() and hashCode() methods in ZipArchiveEntry do not always use the same filename value in their comparisons. In fact if the filename of the entry is set in the ctor it will always treat two ZipArchiveEntries as equal. This will break the offset hashmap whenever there is a hash collision as it will overwrite the previous entry, believeing it to be equal.

      Patch to follow.

        Activity

        Hide
        Stefan Bodewig added a comment -

        ZipArchiveEntry#equals should now compare all data.

        Show
        Stefan Bodewig added a comment - ZipArchiveEntry#equals should now compare all data.
        Hide
        Stefan Bodewig added a comment -

        Ant's codebase didn't have the same problem since Ant's ZipEntry class has a completely different implementation of equals (basically Object#equals).

        I'm not convinced that CC's equals implementation is a good idea since it only takes an entry's name into account and nothing else (size, extra fields, compression mode, last modified date and all that).

        Show
        Stefan Bodewig added a comment - Ant's codebase didn't have the same problem since Ant's ZipEntry class has a completely different implementation of equals (basically Object#equals). I'm not convinced that CC's equals implementation is a good idea since it only takes an entry's name into account and nothing else (size, extra fields, compression mode, last modified date and all that).
        Hide
        Stefan Bodewig added a comment -

        Good catch! Many thanks.

        svn revision 896818 contains an extended version of your patch (which came in just in time to allow me to merge it into Ant's codebase before the 1.8.0 release, great).

        Show
        Stefan Bodewig added a comment - Good catch! Many thanks. svn revision 896818 contains an extended version of your patch (which came in just in time to allow me to merge it into Ant's codebase before the 1.8.0 release, great).
        Hide
        Anon Devs added a comment -

        Patch for COMPRESS-94 to fix the equals() implementation in ZipArchiveEntry.

        Show
        Anon Devs added a comment - Patch for COMPRESS-94 to fix the equals() implementation in ZipArchiveEntry.

          People

          • Assignee:
            Stefan Bodewig
            Reporter:
            Anon Devs
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development