Fixing available() and close() for archive streams.
This builds on work originally submitted to Harmony:
http://issues.apache.org/jira/browse/HARMONY-6210
The approach is to change available() to eagerly set eof to true,
rather than waiting for a read to fail.
diff --git a/libcore/archive/src/main/java/java/util/zip/GZIPInputStream.java b/libcore/archive/src/main/java/java/util/zip/GZIPInputStream.java
index bb84f5b..cc7a019 100644
--- a/libcore/archive/src/main/java/java/util/zip/GZIPInputStream.java
+++ b/libcore/archive/src/main/java/java/util/zip/GZIPInputStream.java
@@ -161,39 +161,55 @@
if (closed) {
throw new IOException(Messages.getString("archive.1E")); //$NON-NLS-1$
}
- if (eof) {
+ // BEGIN android-changed
+ if (eos) {
return -1;
}
// avoid int overflow, check null buffer
- if (off <= buffer.length && nbytes >= 0 && off >= 0
- && buffer.length - off >= nbytes) {
- int val = super.read(buffer, off, nbytes);
- if (val != -1) {
- crc.update(buffer, off, val);
- } else if (!eos) {
- eos = true;
- // Get non-compressed bytes read by fill
- int size = inf.getRemaining();
- final int trailerSize = 8; // crc (4 bytes) + total out (4
- // bytes)
- byte[] b = new byte[trailerSize];
- int copySize = (size > trailerSize) ? trailerSize : size;
-
- System.arraycopy(buf, len - size, b, 0, copySize);
- readFully(b, copySize, trailerSize - copySize);
-
- if (getLong(b, 0) != crc.getValue()) {
- throw new IOException(Messages.getString("archive.20")); //$NON-NLS-1$
- }
- if ((int) getLong(b, 4) != inf.getTotalOut()) {
- throw new IOException(Messages.getString("archive.21")); //$NON-NLS-1$
- }
- }
- return val;
+ if (off > buffer.length || nbytes < 0 || off < 0
+ || buffer.length - off < nbytes) {
+ throw new ArrayIndexOutOfBoundsException();
}
- throw new ArrayIndexOutOfBoundsException();
+
+ int bytesRead;
+ try {
+ bytesRead = super.read(buffer, off, nbytes);
+ } finally {
+ eos = eof; // update eos after every read(), even when it throws
+ }
+
+ if (bytesRead != -1) {
+ crc.update(buffer, off, bytesRead);
+ }
+
+ if (eos) {
+ verifyCrc();
+ }
+
+ return bytesRead;
+ // END android-changed
}
+ // BEGIN android-added
+ private void verifyCrc() throws IOException {
+ // Get non-compressed bytes read by fill
+ int size = inf.getRemaining();
+ final int trailerSize = 8; // crc (4 bytes) + total out (4 bytes)
+ byte[] b = new byte[trailerSize];
+ int copySize = (size > trailerSize) ? trailerSize : size;
+
+ System.arraycopy(buf, len - size, b, 0, copySize);
+ readFully(b, copySize, trailerSize - copySize);
+
+ if (getLong(b, 0) != crc.getValue()) {
+ throw new IOException(Messages.getString("archive.20")); //$NON-NLS-1$
+ }
+ if ((int) getLong(b, 4) != inf.getTotalOut()) {
+ throw new IOException(Messages.getString("archive.21")); //$NON-NLS-1$
+ }
+ }
+ // END android-added
+
private void readFully(byte[] buffer, int offset, int length)
throws IOException {
int result;
diff --git a/libcore/archive/src/main/java/java/util/zip/InflaterInputStream.java b/libcore/archive/src/main/java/java/util/zip/InflaterInputStream.java
index 1fd3602..8a7c86b 100644
--- a/libcore/archive/src/main/java/java/util/zip/InflaterInputStream.java
+++ b/libcore/archive/src/main/java/java/util/zip/InflaterInputStream.java
@@ -53,6 +53,11 @@
boolean closed;
+ /**
+ * True if this stream's last byte has been returned to the user. This
+ * could be because the underlying stream has been exhausted, or if errors
+ * were encountered while inflating that stream.
+ */
boolean eof;
static final int BUF_SIZE = 512;
@@ -165,41 +170,47 @@
return 0;
}
- if (inf.finished()) {
- eof = true;
+ // BEGIN android-changed
+ if (eof) {
return -1;
}
// avoid int overflow, check null buffer
- if (off <= buffer.length && nbytes >= 0 && off >= 0
- && buffer.length - off >= nbytes) {
- do {
- if (inf.needsInput()) {
- fill();
- }
- int result;
- try {
- result = inf.inflate(buffer, off, nbytes);
- } catch (DataFormatException e) {
- if (len == -1) {
- throw new EOFException();
- }
- throw (IOException) (new IOException().initCause(e));
- }
+ if (off > buffer.length || nbytes < 0 || off < 0
+ || buffer.length - off < nbytes) {
+ throw new ArrayIndexOutOfBoundsException();
+ }
+
+ do {
+ if (inf.needsInput()) {
+ fill();
+ }
+ // Invariant: if reading returns -1 or throws, eof must be true.
+ // It may also be true if the next read() should return -1.
+ try {
+ int result = inf.inflate(buffer, off, nbytes);
+ eof = inf.finished();
if (result > 0) {
return result;
- } else if (inf.finished()) {
- eof = true;
+ } else if (eof) {
return -1;
} else if (inf.needsDictionary()) {
+ eof = true;
return -1;
} else if (len == -1) {
+ eof = true;
throw new EOFException();
// If result == 0, fill() and try again
}
- } while (true);
- }
- throw new ArrayIndexOutOfBoundsException();
+ } catch (DataFormatException e) {
+ eof = true;
+ if (len == -1) {
+ throw new EOFException();
+ }
+ throw (IOException) (new IOException().initCause(e));
+ }
+ } while (true);
+ // END android-changed
}
/**
@@ -252,7 +263,9 @@
(rem = nbytes - count) > buf.length ? buf.length
: (int) rem);
if (x == -1) {
- eof = true;
+ // BEGIN android-removed
+ // eof = true;
+ // END android-removed
return count;
}
count += x;
@@ -263,9 +276,18 @@
}
/**
- * Returns whether data can be read from this stream.
+ * Returns 0 when when this stream has exhausted its input; and 1 otherwise.
+ * A result of 1 does not guarantee that further bytes can be returned,
+ * with or without blocking.
*
- * @return 0 if this stream has been closed, 1 otherwise.
+ * <p>Although consistent with the RI, this behavior is inconsistent with
+ * {@link InputStream#available()}, and violates the <a
+ * href="http://en.wikipedia.org/wiki/Liskov_substitution_principle">Liskov
+ * Substitution Principle</a>. This method should not be used.
+ *
+ * @return 0 if no further bytes are available. Otherwise returns 1,
+ * which suggests (but does not guarantee) that additional bytes are
+ * available.
* @throws IOException
* If an error occurs.
*/
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 fd78e4c..f86cbe0 100644
--- a/libcore/archive/src/main/java/java/util/zip/ZipInputStream.java
+++ b/libcore/archive/src/main/java/java/util/zip/ZipInputStream.java
@@ -292,9 +292,6 @@
}
currentEntry.setExtra(e);
}
- // BEGIN android-added
- eof = false;
- // END android-added
return currentEntry;
}
@@ -318,62 +315,56 @@
if (closed) {
throw new IOException(Messages.getString("archive.1E")); //$NON-NLS-1$
}
- // END android-changed
if (inf.finished() || currentEntry == null) {
return -1;
}
// avoid int overflow, check null buffer
- if (start <= buffer.length && length >= 0 && start >= 0
- && buffer.length - start >= length) {
- if (currentEntry.compressionMethod == STORED) {
- int csize = (int) currentEntry.size;
- if (inRead >= csize) {
- // BEGIN android-added
- eof = true;
- // END android-added
- return -1;
- }
- if (lastRead >= len) {
- lastRead = 0;
- if ((len = in.read(buf)) == -1) {
- // BEGIN android-added
- eof = true;
- // END android-added
- return -1;
- }
- entryIn += len;
- }
- // BEGIN android-changed
- int toRead = length > (len - lastRead) ? len - lastRead : length;
- // END android-changed
- if ((csize - inRead) < toRead) {
- toRead = csize - inRead;
- }
- System.arraycopy(buf, lastRead, buffer, start, toRead);
- lastRead += toRead;
- inRead += toRead;
- crc.update(buffer, start, toRead);
- return toRead;
- }
- if (inf.needsInput()) {
- fill();
- if (len > 0) {
- entryIn += len;
- }
- }
- int read = 0;
- try {
- read = inf.inflate(buffer, start, length);
- } catch (DataFormatException e) {
- throw new ZipException(e.getMessage());
- }
- if (read == 0 && inf.finished()) {
+ if (start > buffer.length || length < 0 || start < 0
+ || buffer.length - start < length) {
+ throw new ArrayIndexOutOfBoundsException();
+ }
+
+ if (currentEntry.compressionMethod == STORED) {
+ int csize = (int) currentEntry.size;
+ if (inRead >= csize) {
return -1;
}
- crc.update(buffer, start, read);
- return read;
+ if (lastRead >= len) {
+ lastRead = 0;
+ if ((len = in.read(buf)) == -1) {
+ eof = true;
+ return -1;
+ }
+ entryIn += len;
+ }
+ int toRead = length > (len - lastRead) ? len - lastRead : length;
+ if ((csize - inRead) < toRead) {
+ toRead = csize - inRead;
+ }
+ System.arraycopy(buf, lastRead, buffer, start, toRead);
+ lastRead += toRead;
+ inRead += toRead;
+ crc.update(buffer, start, toRead);
+ return toRead;
}
- throw new ArrayIndexOutOfBoundsException();
+ if (inf.needsInput()) {
+ fill();
+ if (len > 0) {
+ entryIn += len;
+ }
+ }
+ int read;
+ try {
+ read = inf.inflate(buffer, start, length);
+ } catch (DataFormatException e) {
+ throw new ZipException(e.getMessage());
+ }
+ if (read == 0 && inf.finished()) {
+ return -1;
+ }
+ crc.update(buffer, start, read);
+ return read;
+ // END android-changed
}
/**
@@ -416,10 +407,7 @@
if (closed) {
throw new IOException(Messages.getString("archive.1E")); //$NON-NLS-1$
}
- if (currentEntry == null) {
- return 1;
- }
- return super.available();
+ return (currentEntry == null || inRead < currentEntry.size) ? 1 : 0;
// END android-changed
}
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 151eabc..5befa77 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
@@ -154,12 +154,12 @@
assertEquals(actual, desired);
jis.close();
-// try {
-// jis.getNextJarEntry(); //Android implementation does not throw exception
-// fail("IOException expected");
-// } catch (IOException ee) {
-// // expected
-// }
+ try {
+ jis.getNextJarEntry();
+ fail("IOException expected");
+ } catch (IOException ee) {
+ // expected
+ }
File resources = Support_Resources.createTempFolder();
Support_Resources.copyFile(resources, null, "Broken_entry.jar");
@@ -181,8 +181,7 @@
)
public void test_getNextJarEntry_Ex() throws Exception {
final Set<String> desired = new HashSet<String>(Arrays
- .asList(new String[] {
- "foo/", "foo/bar/", "foo/bar/A.class", "Blah.txt"}));
+ .asList("foo/", "foo/bar/", "foo/bar/A.class", "Blah.txt"));
Set<String> actual = new HashSet<String>();
InputStream is = new URL(jarName).openConnection().getInputStream();
JarInputStream jis = new JarInputStream(is);
@@ -195,7 +194,7 @@
jis.close();
try {
- jis.getNextJarEntry(); //Android implementation does not throw exception
+ jis.getNextJarEntry();
fail("IOException expected");
} catch (IOException ee) {
// expected
@@ -275,9 +274,9 @@
)
public void test_JarInputStream_Modified_Manifest_MainAttributes_getNextEntry()
throws IOException {
- String modJarName = Support_Resources
- .getURL("Modified_Manifest_MainAttributes.jar");
- InputStream is = new URL(modJarName).openConnection().getInputStream();
+ String modJarName = Support_Resources.getURL("Modified_Manifest_MainAttributes.jar");
+ InputStream is = new URL(modJarName).openConnection()
+ .getInputStream();
JarInputStream jin = new JarInputStream(is, true);
assertEquals("META-INF/TESTROOT.SF", jin.getNextEntry().getName());
diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/DeflaterOutputStreamTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/DeflaterOutputStreamTest.java
index ed7238c..738f610 100644
--- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/DeflaterOutputStreamTest.java
+++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/DeflaterOutputStreamTest.java
@@ -211,21 +211,22 @@
)
public void test_close() throws Exception {
File f1 = File.createTempFile("close", ".tst");
- FileOutputStream fos = new FileOutputStream(f1);
- DeflaterOutputStream dos = new DeflaterOutputStream(fos);
- byte byteArray[] = {1, 3, 4, 6};
- dos.write(byteArray);
- FileInputStream fis = new FileInputStream(f1);
- InflaterInputStream iis = new InflaterInputStream(fis);
+ InflaterInputStream iis = new InflaterInputStream(new FileInputStream(f1));
try {
iis.read();
fail("EOFException Not Thrown");
} catch (EOFException e) {
}
+ FileOutputStream fos = new FileOutputStream(f1);
+ DeflaterOutputStream dos = new DeflaterOutputStream(fos);
+ byte byteArray[] = {1, 3, 4, 6};
+ dos.write(byteArray);
dos.close();
+ iis = new InflaterInputStream(new FileInputStream(f1));
+
// Test to see if the finish method wrote the bytes to the file.
assertEquals("Incorrect Byte Returned.", 1, iis.read());
assertEquals("Incorrect Byte Returned.", 3, iis.read());
diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterInputStreamTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterInputStreamTest.java
index 2de996e..707f13b 100644
--- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterInputStreamTest.java
+++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/InflaterInputStreamTest.java
@@ -32,6 +32,8 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.FileOutputStream;
+import java.io.EOFException;
import java.util.zip.DeflaterOutputStream;
import java.util.zip.Inflater;
import java.util.zip.InflaterInputStream;
@@ -208,6 +210,42 @@
}
}
+ public void testAvailableNonEmptySource() throws Exception {
+ // this byte[] is a deflation of these bytes: { 1, 3, 4, 6 }
+ byte[] deflated = { 72, -119, 99, 100, 102, 97, 3, 0, 0, 31, 0, 15, 0 };
+ InputStream in = new InflaterInputStream(new ByteArrayInputStream(deflated));
+ // InflaterInputStream.available() returns either 1 or 0, even though
+ // that contradicts the behavior defined in InputStream.available()
+ assertEquals(1, in.read());
+ assertEquals(1, in.available());
+ assertEquals(3, in.read());
+ assertEquals(1, in.available());
+ assertEquals(4, in.read());
+ assertEquals(1, in.available());
+ assertEquals(6, in.read());
+ assertEquals(0, in.available());
+ assertEquals(-1, in.read());
+ assertEquals(-1, in.read());
+ }
+
+ public void testAvailableSkip() throws Exception {
+ // this byte[] is a deflation of these bytes: { 1, 3, 4, 6 }
+ byte[] deflated = { 72, -119, 99, 100, 102, 97, 3, 0, 0, 31, 0, 15, 0 };
+ InputStream in = new InflaterInputStream(new ByteArrayInputStream(deflated));
+ assertEquals(1, in.available());
+ assertEquals(4, in.skip(4));
+ assertEquals(0, in.available());
+ }
+
+ public void testAvailableEmptySource() throws Exception {
+ // this byte[] is a deflation of the empty file
+ byte[] deflated = { 120, -100, 3, 0, 0, 0, 0, 1 };
+ InputStream in = new InflaterInputStream(new ByteArrayInputStream(deflated));
+ assertEquals(-1, in.read());
+ assertEquals(-1, in.read());
+ assertEquals(0, in.available());
+ }
+
/**
* @tests java.util.zip.InflaterInputStream#read(byte[], int, int)
*/
@@ -471,12 +509,11 @@
InflaterInputStream iis = new InflaterInputStream(is);
int available;
- int read;
for (int i = 0; i < 11; i++) {
- read = iis.read();
+ iis.read();
available = iis.available();
- if (read == -1) {
- assertEquals("Bytes Available Should Return 0 ", 0, available);
+ if (available == 0) {
+ assertEquals("Expected no more bytes to read", -1, iis.read());
} else {
assertEquals("Bytes Available Should Return 1.", 1, available);
}
diff --git a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipFileTest.java b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipFileTest.java
index c9e7bb8..b025e11 100644
--- a/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipFileTest.java
+++ b/libcore/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/ZipFileTest.java
@@ -18,7 +18,6 @@
package org.apache.harmony.archive.tests.java.util.zip;
import dalvik.annotation.KnownFailure;
-import dalvik.annotation.BrokenTest;
import dalvik.annotation.TestLevel;
import dalvik.annotation.TestTargetClass;
import dalvik.annotation.TestTargetNew;
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 1d6c339..c5efedc 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
@@ -17,7 +17,6 @@
package org.apache.harmony.archive.tests.java.util.zip;
-import dalvik.annotation.KnownFailure;
import dalvik.annotation.TestLevel;
import dalvik.annotation.TestTargetClass;
import dalvik.annotation.TestTargetNew;
@@ -384,19 +383,19 @@
long entrySize = entry.getSize();
assertTrue("Entry size was < 1", entrySize > 0);
int i = 0;
- for (i = 0; i < entrySize; i++) {
+ while (zis1.available() > 0) {
zis1.skip(1);
- if (zis1.available() == 0) break;
+ i++;
}
if (i != entrySize) {
fail("ZipInputStream.available or ZipInputStream.skip does not " +
"working properly. Only skipped " + i +
" bytes instead of " + entrySize);
}
- zis1.skip(1);
- assertTrue(zis1.available() == 0);
+ assertEquals(0, zis1.skip(1));
+ assertEquals(0, zis1.available());
zis1.closeEntry();
- assertFalse(zis.available() == 0);
+ assertEquals(1, zis.available());
zis1.close();
try {
zis1.available();