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.m...@lookout.com> Authored: Fri Oct 28 08:03:48 2016 -0400 Committer: Stefan Bodewig <bode...@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.
