From ef164bf196538c04f499dcbb49a389c70ff5601a Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 26 Feb 2014 14:20:22 +0000 Subject: Improve error message when attempting to open an empty zip The bug report states that the behaviour of ZipFile has changed between 4.3 and 4.4 when opening an empty zip file. I did some investigation and it appears as though the reference implementation also throws an exception in that case so I decided that 4.4 is working as designed. However, to make it clearer I made a minor change to explicitly check for an empty zip file so that we can report a slightly improved message explaining that they are not supported. I added a test for various forms of empty streams and zip files and removed part of an older test from harmony tests as it is no longer necessary. Bug: https://code.google.com/p/android/issues/detail?id=65380 Change-Id: I1f2fcbf6bbaedb7dbccf8dd4f1cec4e330274524 --- .../tests/java/util/zip/ZipOutputStreamTest.java | 7 ----- luni/src/main/java/java/util/zip/ZipFile.java | 3 +++ .../java/libcore/java/util/zip/ZipFileTest.java | 29 ++++++++++++++++++++- .../libcore/java/util/zip/ZipInputStreamTest.java | 19 ++++++++++++-- .../libcore/java/util/zip/ZipOutputStreamTest.java | 24 ++++++++++++++--- .../tests/resources/java/util/zip/EmptyArchive.zip | Bin 0 -> 22 bytes 6 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 luni/src/test/resources/tests/resources/java/util/zip/EmptyArchive.zip diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/zip/ZipOutputStreamTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/zip/ZipOutputStreamTest.java index a9e43d1..7f42fa8 100644 --- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/zip/ZipOutputStreamTest.java +++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/zip/ZipOutputStreamTest.java @@ -41,13 +41,6 @@ public class ZipOutputStreamTest extends junit.framework.TestCase { * java.util.zip.ZipOutputStream#close() */ public void test_close() throws Exception { - try { - zos.close(); - fail("Close on empty stream failed to throw exception"); - } catch (ZipException e) { - // expected - } - zos = new ZipOutputStream(bos); zos.putNextEntry(new ZipEntry("XX")); zos.closeEntry(); diff --git a/luni/src/main/java/java/util/zip/ZipFile.java b/luni/src/main/java/java/util/zip/ZipFile.java index 40036cf..4380281 100644 --- a/luni/src/main/java/java/util/zip/ZipFile.java +++ b/luni/src/main/java/java/util/zip/ZipFile.java @@ -357,6 +357,9 @@ public class ZipFile implements Closeable, ZipConstants { raf.seek(0); final int headerMagic = Integer.reverseBytes(raf.readInt()); + if (headerMagic == ENDSIG) { + throw new ZipException("Empty zip archive not supported"); + } if (headerMagic != LOCSIG) { throw new ZipException("Not a zip archive"); } diff --git a/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java b/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java index 60af4d0..8afc223 100644 --- a/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java +++ b/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java @@ -21,7 +21,6 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; -import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; import java.util.Enumeration; @@ -34,6 +33,8 @@ import java.util.zip.ZipInputStream; import java.util.zip.ZipOutputStream; import junit.framework.TestCase; +import tests.support.resource.Support_Resources; + public final class ZipFileTest extends TestCase { /** * Exercise Inflater's ability to refill the zlib's input buffer. As of this @@ -468,4 +469,30 @@ public final class ZipFileTest extends TestCase { out.closeEntry(); out.close(); } + + /** + * RI does not allow reading of an empty zip using a {@link ZipFile}. + */ + public void testConstructorFailsWhenReadingEmptyZipArchive() throws IOException { + + File resources = Support_Resources.createTempFolder(); + File emptyZip = Support_Resources.copyFile( + resources, "java/util/zip", "EmptyArchive.zip"); + + try { + // The following should fail with an exception but if it doesn't then we need to clean + // up the resource so we need a reference to it. + ZipFile zipFile = new ZipFile(emptyZip); + + // Clean up the resource. + try { + zipFile.close(); + } catch (Exception e) { + // Ignore + } + fail(); + } catch (ZipException expected) { + // expected + } + } } diff --git a/luni/src/test/java/libcore/java/util/zip/ZipInputStreamTest.java b/luni/src/test/java/libcore/java/util/zip/ZipInputStreamTest.java index cb98322..3d6e600 100644 --- a/luni/src/test/java/libcore/java/util/zip/ZipInputStreamTest.java +++ b/luni/src/test/java/libcore/java/util/zip/ZipInputStreamTest.java @@ -20,15 +20,16 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; import java.util.Arrays; import java.util.Random; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; -import java.util.zip.ZipOutputStream; import junit.framework.TestCase; +import tests.support.resource.Support_Resources; + public final class ZipInputStreamTest extends TestCase { + public void testShortMessage() throws IOException { byte[] data = "Hello World".getBytes("UTF-8"); byte[] zipped = ZipOutputStreamTest.zip("short", data); @@ -59,4 +60,18 @@ public final class ZipInputStreamTest extends TestCase { in.close(); return out.toByteArray(); } + + /** + * Reference implementation allows reading of empty zip using a {@link ZipInputStream}. + */ + public void testReadEmpty() throws IOException { + InputStream emptyZipIn = Support_Resources.getStream("java/util/zip/EmptyArchive.zip"); + ZipInputStream in = new ZipInputStream(emptyZipIn); + try { + ZipEntry entry = in.getNextEntry(); + assertNull("An empty zip has no entries", entry); + } finally { + in.close(); + } + } } diff --git a/luni/src/test/java/libcore/java/util/zip/ZipOutputStreamTest.java b/luni/src/test/java/libcore/java/util/zip/ZipOutputStreamTest.java index e7c518f..dc80512 100644 --- a/luni/src/test/java/libcore/java/util/zip/ZipOutputStreamTest.java +++ b/luni/src/test/java/libcore/java/util/zip/ZipOutputStreamTest.java @@ -16,15 +16,15 @@ package libcore.java.util.zip; -import java.io.ByteArrayInputStream; +import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; import java.util.Arrays; import java.util.Random; import java.util.zip.ZipEntry; -import java.util.zip.ZipInputStream; +import java.util.zip.ZipException; import java.util.zip.ZipOutputStream; import junit.framework.TestCase; @@ -60,4 +60,20 @@ public final class ZipOutputStreamTest extends TestCase { zippedOut.close(); return bytesOut.toByteArray(); } + + /** + * Reference implementation does NOT allow writing of an empty zip using a + * {@link ZipOutputStream}. + */ + public void testCreateEmpty() throws IOException { + File result = File.createTempFile("ZipFileTest", "zip"); + ZipOutputStream out = + new ZipOutputStream(new BufferedOutputStream(new FileOutputStream(result))); + try { + out.close(); + fail("Close on empty stream failed to throw exception"); + } catch (ZipException e) { + // expected + } + } } diff --git a/luni/src/test/resources/tests/resources/java/util/zip/EmptyArchive.zip b/luni/src/test/resources/tests/resources/java/util/zip/EmptyArchive.zip new file mode 100644 index 0000000..15cb0ec Binary files /dev/null and b/luni/src/test/resources/tests/resources/java/util/zip/EmptyArchive.zip differ -- cgit v1.1