commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bode...@apache.org
Subject [1/2] commons-compress git commit: COMPRESS-367 Throw ZipException on invalid entry with ZipArchiveInputStream
Date Fri, 09 Dec 2016 22:03:54 GMT
Repository: commons-compress
Updated Branches:
  refs/heads/master e0c83d24e -> 726ba6f0f


COMPRESS-367 Throw ZipException on invalid entry with ZipArchiveInputStream

ZipArchiveInputStream.getNextZipEntry() throws a ZipException rather
than returning null if an invalid entry is encountered in order to
differentiate between "no more entries" and "error" conditions.


Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/f0153448
Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/f0153448
Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/f0153448

Branch: refs/heads/master
Commit: f015344879f1bc3505bc616fc7ae517cf9f60838
Parents: e0c83d2
Author: Mike Mole <michael.mole@lookout.com>
Authored: Fri Oct 28 08:03:48 2016 -0400
Committer: Stefan Bodewig <bodewig@apache.org>
Committed: Fri Dec 9 23:01:42 2016 +0100

----------------------------------------------------------------------
 .../archivers/zip/ZipArchiveInputStream.java    |  3 ++-
 .../commons/compress/archivers/ZipTestCase.java | 26 +++++++++++++-------
 .../zip/ZipArchiveInputStreamTest.java          | 21 ++++++++++++++++
 src/test/resources/invalid-zip.zip              |  2 ++
 4 files changed, 42 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-compress/blob/f0153448/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
index 5d4c0a8..51b1930 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
@@ -244,9 +244,10 @@ public class ZipArchiveInputStream extends ArchiveInputStream {
         if (sig.equals(ZipLong.CFH_SIG) || sig.equals(ZipLong.AED_SIG)) {
             hitCentralDirectory = true;
             skipRemainderOfArchive();
+            return null;
         }
         if (!sig.equals(ZipLong.LFH_SIG)) {
-            return null;
+            throw new ZipException(String.format("Unexpected record signature: 0X%X", sig.getValue()));
         }
 
         int off = WORD;

http://git-wip-us.apache.org/repos/asf/commons-compress/blob/f0153448/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java b/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
index c54cbae..7988e23 100644
--- a/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
+++ b/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
@@ -31,6 +31,7 @@ import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
 
 import org.apache.commons.compress.AbstractTestCase;
 import org.apache.commons.compress.archivers.zip.Zip64Mode;
@@ -243,6 +244,7 @@ public final class ZipTestCase extends AbstractTestCase {
         final File input = getFile("OSX_ArchiveWithNestedArchive.zip");
 
         final List<String> results = new ArrayList<>();
+        final List<ZipException> expectedExceptions = new ArrayList<>();
 
         final InputStream is = new FileInputStream(input);
         ArchiveInputStream in = null;
@@ -250,15 +252,20 @@ public final class ZipTestCase extends AbstractTestCase {
             in = new ArchiveStreamFactory().createArchiveInputStream("zip", is);
 
             ZipArchiveEntry entry = null;
-            while((entry = (ZipArchiveEntry)in.getNextEntry()) != null) {
+            while ((entry = (ZipArchiveEntry) in.getNextEntry()) != null) {
                 results.add(entry.getName());
 
                 final ArchiveInputStream nestedIn = new ArchiveStreamFactory().createArchiveInputStream("zip",
in);
-                ZipArchiveEntry nestedEntry = null;
-                while((nestedEntry = (ZipArchiveEntry)nestedIn.getNextEntry()) != null) {
-                    results.add(nestedEntry.getName());
+                try {
+                    ZipArchiveEntry nestedEntry = null;
+                    while ((nestedEntry = (ZipArchiveEntry) nestedIn.getNextEntry()) != null)
{
+                        results.add(nestedEntry.getName());
+                    }
+                } catch (ZipException ex) {
+                    // expected since you cannot create a final ArchiveInputStream from test3.xml
+                    expectedExceptions.add(ex);
                 }
-               // nested stream must not be closed here
+                // nested stream must not be closed here
             }
         } finally {
             if (in != null) {
@@ -267,10 +274,11 @@ public final class ZipTestCase extends AbstractTestCase {
         }
         is.close();
 
-        results.contains("NestedArchiv.zip");
-        results.contains("test1.xml");
-        results.contains("test2.xml");
-        results.contains("test3.xml");
+        assertTrue(results.contains("NestedArchiv.zip"));
+        assertTrue(results.contains("test1.xml"));
+        assertTrue(results.contains("test2.xml"));
+        assertTrue(results.contains("test3.xml"));
+        assertEquals(1, expectedExceptions.size());
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/commons-compress/blob/f0153448/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
index 297de9a..ea087c4 100644
--- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
@@ -30,6 +30,7 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Arrays;
+import java.util.zip.ZipException;
 
 import org.apache.commons.compress.utils.IOUtils;
 import org.junit.Test;
@@ -227,6 +228,26 @@ public class ZipArchiveInputStreamTest {
         }
     }
 
+    /**
+     * <code>getNextZipEntry()</code> should throw a <code>ZipException</code>
rather than return
+     * <code>null</code> when an unexpected structure is encountered.
+     */
+    @Test
+    public void testThrowOnInvalidEntry() throws Exception {
+        final InputStream is = ZipArchiveInputStreamTest.class
+                .getResourceAsStream("/invalid-zip.zip");
+        final ZipArchiveInputStream zip = new ZipArchiveInputStream(is);
+
+        try {
+            zip.getNextZipEntry();
+            fail("IOException expected");
+        } catch (ZipException expected) {
+            assertTrue(expected.getMessage().contains("Unexpected record signature"));
+        } finally {
+            zip.close();
+        }
+    }
+
     private static byte[] readEntry(ZipArchiveInputStream zip, ZipArchiveEntry zae) throws
IOException {
         final int len = (int)zae.getSize();
         final byte[] buff = new byte[len];

http://git-wip-us.apache.org/repos/asf/commons-compress/blob/f0153448/src/test/resources/invalid-zip.zip
----------------------------------------------------------------------
diff --git a/src/test/resources/invalid-zip.zip b/src/test/resources/invalid-zip.zip
new file mode 100644
index 0000000..ff6f1ec
--- /dev/null
+++ b/src/test/resources/invalid-zip.zip
@@ -0,0 +1,2 @@
+This is not really a valid zip file even though it has the zip extension.
+ZipArchiveInputStream.getNextZipEntry() should throw an exception.


Mime
View raw message