From 05c0836f27bd73d74e2b47ee86cd925fc5548ebc Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Tue, 15 Sep 2009 14:30:29 -0700 Subject: [PATCH] Update archive to the r802921 from Harmony. This overdue update includes fixes for some Eclair bugs that I originally submitted to Harmony: 1876785, 1635982, 1635998 --- .../src/main/java/java/util/jar/Pack200.java | 4 +- .../src/main/java/java/util/zip/Inflater.java | 41 +++++++------- .../main/java/java/util/zip/ZipInputStream.java | 63 +++++++++++++++++----- .../tests/java/util/jar/JarInputStreamTest.java | 30 ++++------- .../archive/tests/java/util/zip/InflaterTest.java | 11 ---- .../tests/java/util/zip/ZipInputStreamTest.java | 28 +++++++++- .../tests/java/util/zip/ZipOutputStreamTest.java | 2 +- 7 files changed, 109 insertions(+), 70 deletions(-) diff --git a/libcore/archive/src/main/java/java/util/jar/Pack200.java b/libcore/archive/src/main/java/java/util/jar/Pack200.java index 8145fa12f..3e7081d2e 100644 --- a/libcore/archive/src/main/java/java/util/jar/Pack200.java +++ b/libcore/archive/src/main/java/java/util/jar/Pack200.java @@ -57,7 +57,7 @@ public abstract class Pack200 { public Object run() { String className = System .getProperty(SYSTEM_PROPERTY_PACKER, - "org.apache.harmony.archive.internal.pack200.Pack200PackerAdapter"); //$NON-NLS-1$ + "org.apache.harmony.pack200.Pack200PackerAdapter"); //$NON-NLS-1$ try { // TODO Not sure if this will cause problems with // loading the packer @@ -87,7 +87,7 @@ public abstract class Pack200 { public Object run() { String className = System .getProperty(SYSTEM_PROPERTY_UNPACKER, - "org.apache.harmony.archive.internal.pack200.Pack200UnpackerAdapter");//$NON-NLS-1$ + "org.apache.harmony.unpack200.Pack200UnpackerAdapter");//$NON-NLS-1$ try { return ClassLoader.getSystemClassLoader() .loadClass(className).newInstance(); diff --git a/libcore/archive/src/main/java/java/util/zip/Inflater.java b/libcore/archive/src/main/java/java/util/zip/Inflater.java index cb1ce6875..048d95979 100644 --- a/libcore/archive/src/main/java/java/util/zip/Inflater.java +++ b/libcore/archive/src/main/java/java/util/zip/Inflater.java @@ -51,7 +51,9 @@ public class Inflater { private boolean finished; // Set by the inflateImpl native - private boolean gotFirstByte = false; + // BEGIN android-removed + // private boolean gotFirstHeaderByte; + // END android-removed int inLength; @@ -59,7 +61,9 @@ public class Inflater { private boolean needsDictionary; // Set by the inflateImpl native - private boolean pass_magic_number_check = true; + // BEGIN android-removed + // private boolean pass_magic_number_check = true; + // END android-removed private long streamHandle = -1; @@ -82,6 +86,9 @@ public class Inflater { */ public Inflater(boolean noHeader) { streamHandle = createStream(noHeader); + // BEGIN android-removed + // gotFirstHeaderByte = noHeader; + // END android-removed } private native long createStream(boolean noHeader1); @@ -250,9 +257,11 @@ public class Inflater { throw new IllegalStateException(); } - if (!pass_magic_number_check) { - throw new DataFormatException(); - } + // BEGIN android-removed + // if (!pass_magic_number_check) { + // throw new DataFormatException(); + // } + // END android-removed if (needsInput()) { return 0; @@ -398,21 +407,13 @@ public class Inflater { } else { throw new ArrayIndexOutOfBoundsException(); } - // BEGIN android-note - // Note: pass_magic_number_check is set to false when setInput is - // called for the first time and for a single byte. - // Since setInput is called only by InflaterInputStream.fill - // with an arbitrary byte len this check seems quite useless. - // FIXME: We should find out whether the first byte has to be the magic - // number in all cases and correct the check as well as place it - // in setFileInput accordingly. - // And at a first glance it doesn't look like the first byte has - // to be 120. - // END android-note - if (!gotFirstByte && nbytes > 0) { - pass_magic_number_check = (buf[off] == MAGIC_NUMBER || nbytes > 1); - gotFirstByte = true; - } + + // BEGIN android-removed + // if (!gotFirstHeaderByte && nbytes > 0) { + // pass_magic_number_check = (buf[off] == MAGIC_NUMBER); + // gotFirstHeaderByte = true; + //} + // END android-removed } // BEGIN android-added diff --git a/libcore/archive/src/main/java/java/util/zip/ZipInputStream.java b/libcore/archive/src/main/java/java/util/zip/ZipInputStream.java index 1a35b1c13..fd78e4cad 100644 --- a/libcore/archive/src/main/java/java/util/zip/ZipInputStream.java +++ b/libcore/archive/src/main/java/java/util/zip/ZipInputStream.java @@ -125,8 +125,24 @@ public class ZipInputStream extends InflaterInputStream implements ZipConstants return; } } + + /* + * The following code is careful to leave the ZipInputStream in a + * consistent state, even when close() results in an exception. It does + * so by: + * - pushing bytes back into the source stream + * - reading a data descriptor footer from the source stream + * - resetting fields that manage the entry being closed + */ + // Ensure all entry bytes are read - skip(Long.MAX_VALUE); + Exception failure = null; + try { + skip(Long.MAX_VALUE); + } catch (Exception e) { + failure = e; + } + int inB, out; if (currentEntry.compressionMethod == DEFLATED) { inB = inf.getTotalIn(); @@ -135,12 +151,38 @@ public class ZipInputStream extends InflaterInputStream implements ZipConstants inB = inRead; out = inRead; } - int diff = 0; + int diff = entryIn - inB; // Pushback any required bytes - if ((diff = entryIn - inB) != 0) { + if (diff != 0) { ((PushbackInputStream) in).unread(buf, len - diff, diff); } + try { + readAndVerifyDataDescriptor(inB, out); + } catch (Exception e) { + if (failure == null) { // otherwise we're already going to throw + failure = e; + } + } + + inf.reset(); + lastRead = inRead = entryIn = len = 0; + crc.reset(); + currentEntry = null; + + if (failure != null) { + if (failure instanceof IOException) { + throw (IOException) failure; + } else if (failure instanceof RuntimeException) { + throw (RuntimeException) failure; + } + AssertionError error = new AssertionError(); + error.initCause(failure); + throw error; + } + } + + private void readAndVerifyDataDescriptor(int inB, int out) throws IOException { if (hasDD) { in.read(hdrBuf, 0, EXTHDR); if (getLong(hdrBuf, 0) != EXTSIG) { @@ -156,26 +198,19 @@ public class ZipInputStream extends InflaterInputStream implements ZipConstants if (currentEntry.compressedSize != inB || currentEntry.size != out) { throw new ZipException(Messages.getString("archive.21")); //$NON-NLS-1$ } - - inf.reset(); - lastRead = inRead = entryIn = len = 0; - crc.reset(); - currentEntry = null; } /** - * Reads the next entry from this {@code ZipInputStream}. + * Reads the next entry from this {@code ZipInputStream} or {@code null} if + * no more entries are present. * * @return the next {@code ZipEntry} contained in the input stream. * @throws IOException - * if the stream is not positioned at the beginning of an entry - * or if an other {@code IOException} occurs. + * if an {@code IOException} occurs. * @see ZipEntry */ public ZipEntry getNextEntry() throws IOException { - if (currentEntry != null) { - closeEntry(); - } + closeEntry(); if (entriesEnd) { return null; } diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/jar/JarInputStreamTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/jar/JarInputStreamTest.java index 64e0e1aa1..151eabc91 100644 --- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/jar/JarInputStreamTest.java +++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/jar/JarInputStreamTest.java @@ -179,7 +179,6 @@ public class JarInputStreamTest extends junit.framework.TestCase { method = "getNextJarEntry", args = {} ) - @KnownFailure("IOException not thrown when using getNextJarEntry() after close().") public void test_getNextJarEntry_Ex() throws Exception { final Set desired = new HashSet(Arrays .asList(new String[] { @@ -280,25 +279,17 @@ public class JarInputStreamTest extends junit.framework.TestCase { .getURL("Modified_Manifest_MainAttributes.jar"); InputStream is = new URL(modJarName).openConnection().getInputStream(); JarInputStream jin = new JarInputStream(is, true); - ZipEntry zipEntry = null; - final int indexofDSA = 2; - final int totalEntries = 4; - int count = 0; - while (count == 0 || zipEntry != null) { - count++; - try { - zipEntry = jin.getNextEntry(); - if (count == indexofDSA + 1) { - fail("Should throw Security Exception"); - } - } catch (SecurityException e) { - if (count != indexofDSA + 1) { - throw e; - } - } + assertEquals("META-INF/TESTROOT.SF", jin.getNextEntry().getName()); + assertEquals("META-INF/TESTROOT.DSA", jin.getNextEntry().getName()); + try { + jin.getNextEntry(); + fail(); + } catch (SecurityException expected) { } - assertEquals(totalEntries + 2, count); + assertEquals("META-INF/", jin.getNextEntry().getName()); + assertEquals("Test.class", jin.getNextEntry().getName()); + assertNull(jin.getNextEntry()); jin.close(); } @@ -542,7 +533,6 @@ public class JarInputStreamTest extends junit.framework.TestCase { method = "close", args = {} ) - @KnownFailure("The behaviour is different from RI, but not neccessarily wrong. However a strange exception message is given anyway!") public void test_closeAfterException() throws Exception { File resources = Support_Resources.createTempFolder(); Support_Resources.copyFile(resources, null, "Broken_entry.jar"); @@ -555,7 +545,7 @@ public class JarInputStreamTest extends junit.framework.TestCase { } catch (ZipException ee) { // expected } - jis.close(); // Android throws exception here, but RI throws when getNextEntry/read/skip are called. + jis.close(); try { jis.getNextEntry(); fail("IOException expected"); diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterTest.java index 6039c5b21..cd5d538fa 100644 --- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterTest.java +++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterTest.java @@ -862,17 +862,6 @@ public class InflaterTest extends junit.framework.TestCase { byte[] b = new byte[1024]; assertEquals(0, inflater.inflate(b)); inflater.end(); - - // Regression for HARMONY-2510 - inflater = new Inflater(); - byte[] input = new byte[] {-1}; - inflater.setInput(input); - try { - inflater.inflate(b); - fail("should throw DataFormateException"); - } catch (DataFormatException e) { - // expected - } } /** diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipInputStreamTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipInputStreamTest.java index ac332faad..1d6c339f5 100644 --- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipInputStreamTest.java +++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipInputStreamTest.java @@ -30,6 +30,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.util.zip.ZipEntry; @@ -174,8 +175,6 @@ public class ZipInputStreamTest extends TestCase { method = "close", args = {} ) - @KnownFailure("after an exception has been thrown wile reading a " - + "call to close also throws an exception.") public void test_closeAfterException() throws Exception { File resources = Support_Resources.createTempFolder(); Support_Resources.copyFile(resources, null, "Broken_manifest.jar"); @@ -281,6 +280,31 @@ public class ZipInputStreamTest extends TestCase { } } + @TestTargetNew( + level = TestLevel.PARTIAL_COMPLETE, + method = "read", + args = {byte[].class, int.class, int.class} + ) + public void testReadOneByteAtATime() throws IOException { + InputStream in = new FilterInputStream(Support_Resources.getStream("hyts_ZipFile.zip")) { + @Override + public int read(byte[] buffer, int offset, int count) throws IOException { + return super.read(buffer, offset, 1); // one byte at a time + } + + @Override + public int read(byte[] buffer) throws IOException { + return super.read(buffer, 0, 1); // one byte at a time + } + }; + + zis = new ZipInputStream(in); + while ((zentry = zis.getNextEntry()) != null) { + zentry.getName(); + } + zis.close(); + } + /** * @tests java.util.zip.ZipInputStream#skip(long) */ diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipOutputStreamTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipOutputStreamTest.java index 8ca551df4..0398f03fd 100644 --- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipOutputStreamTest.java +++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipOutputStreamTest.java @@ -86,7 +86,7 @@ public class ZipOutputStreamTest extends junit.framework.TestCase { ZipEntry ze = new ZipEntry("testEntry"); ze.setTime(System.currentTimeMillis()); zos.putNextEntry(ze); - zos.write("Hello World".getBytes()); + zos.write("Hello World".getBytes("UTF-8")); zos.closeEntry(); assertTrue("closeEntry failed to update required fields", ze.getSize() == 11 && ze.getCompressedSize() == 13); -- 2.11.0