From: Tim Allison
+ * Note that the implementation uses {@link InputStream#read(byte[], int, int)} rather
+ * than delegating to {@link InputStream#skip(long)}.
+ * This means that the method may be considerably less efficient than using the actual skip implementation,
+ * this is done to guarantee that the correct number of bytes are skipped.
+ *
+ * This mimics POI's {@link #readFully(InputStream, byte[])}.
+ * If the end of file is reached before any bytes are read, returns -1. If
+ * the end of the file is reached after some bytes are read, returns the
+ * number of bytes read. If the end of the file isn't reached before len
+ * bytes have been read, will return len bytes.
+ * Copied nearly verbatim from commons-io 41a3e9c + *
+ * + * @param input byte stream to skip + * @param toSkip number of bytes to skip. + * @return number of bytes actually skipped. + * @throws IOException if there is a problem reading the file + * @throws IllegalArgumentException if toSkip is negative + * @see InputStream#skip(long) * - * @param in inputstream - * @param len length to skip - * @return number of bytes skipped - * @throws IOException on IOException */ - public static long skipFully(InputStream in, long len) throws IOException { - long total = 0; - while (true) { - long toSkip = len-total; - //check that the stream has the toSkip available - //FileInputStream can mis-report 20k skipped on a 10k file - if (toSkip > in.available()) { - return -1L; - } - long got = in.skip(len-total); - if (got < 0) { - return -1L; - } else if (got == 0) { - got = fallBackToReadFully(len-total, in); - if (got < 0) { - return -1L; - } - } - total += got; - if (total == len) { - return total; - } + public static long skipFully(final InputStream input, final long toSkip) throws IOException { + if (toSkip < 0) { + throw new IllegalArgumentException("Skip count must be non-negative, actual: " + toSkip); } - } - - //an InputStream can return 0 whether or not it hits EOF - //if it returns 0, back off to readFully to test for -1 - private static long fallBackToReadFully(long lenToRead, InputStream in) throws IOException { - byte[] buffer = new byte[8192]; - long readSoFar = 0; - - while (true) { - int toSkip = (lenToRead > Integer.MAX_VALUE || - (lenToRead-readSoFar) > buffer.length) ? buffer.length : (int)(lenToRead-readSoFar); - long readNow = readFully(in, buffer, 0, toSkip); - if (readNow < toSkip) { - return -1L; - } - readSoFar += readNow; - if (readSoFar == lenToRead) { - return readSoFar; + if (toSkip == 0) { + return 0L; + } + /* + * N.B. no need to synchronize this because: - we don't care if the buffer is created multiple times (the data + * is ignored) - we always use the same size buffer, so if it it is recreated it will still be OK (if the buffer + * size were variable, we would need to synch. to ensure some other thread did not create a smaller one) + */ + if (SKIP_BYTE_BUFFER == null) { + SKIP_BYTE_BUFFER = new byte[SKIP_BUFFER_SIZE]; + } + long remain = toSkip; + while (remain > 0) { + // See https://issues.apache.org/jira/browse/IO-203 for why we use read() rather than delegating to skip() + final long n = input.read(SKIP_BYTE_BUFFER, 0, (int) Math.min(remain, SKIP_BUFFER_SIZE)); + if (n < 0) { // EOF + break; } + remain -= n; + } + if (toSkip == remain) { + return -1L; } + return toSkip - remain; } + } diff --git a/src/scratchpad/src/org/apache/poi/hemf/record/HemfCommentRecord.java b/src/scratchpad/src/org/apache/poi/hemf/record/HemfCommentRecord.java index 51efb2f2b0..2e8c17d7ad 100644 --- a/src/scratchpad/src/org/apache/poi/hemf/record/HemfCommentRecord.java +++ b/src/scratchpad/src/org/apache/poi/hemf/record/HemfCommentRecord.java @@ -89,8 +89,15 @@ public class HemfCommentRecord implements HemfRecord { int recordSize = (int)remainingRecordSize; byte[] arr = new byte[dataSize+initialBytes.length]; System.arraycopy(initialBytes,0,arr, 0, initialBytes.length); - IOUtils.readFully(leis, arr, initialBytes.length, dataSize); - IOUtils.skipFully(leis, recordSize-dataSize); + long read = IOUtils.readFully(leis, arr, initialBytes.length, dataSize); + if (read != dataSize) { + throw new RecordFormatException("InputStream ended before full record could be read"); + } + long toSkip = recordSize-dataSize; + long skipped = IOUtils.skipFully(leis, toSkip); + if (toSkip != skipped) { + throw new RecordFormatException("InputStream ended before full record could be read"); + } return arr; } @@ -103,8 +110,16 @@ public class HemfCommentRecord implements HemfRecord { } byte[] arr = new byte[(int)dataSize]; - IOUtils.readFully(leis, arr); - IOUtils.skipFully(leis, recordSize-dataSize); + + long read = IOUtils.readFully(leis, arr); + if (read != dataSize) { + throw new RecordFormatException("InputStream ended before full record could be read"); + } + long toSkip = recordSize-dataSize; + long skipped = IOUtils.skipFully(leis, recordSize-dataSize); + if (toSkip != skipped) { + throw new RecordFormatException("InputStream ended before full record could be read"); + } return arr; } diff --git a/src/scratchpad/src/org/apache/poi/hemf/record/UnimplementedHemfRecord.java b/src/scratchpad/src/org/apache/poi/hemf/record/UnimplementedHemfRecord.java index a951e0ec5b..6f3ded48d5 100644 --- a/src/scratchpad/src/org/apache/poi/hemf/record/UnimplementedHemfRecord.java +++ b/src/scratchpad/src/org/apache/poi/hemf/record/UnimplementedHemfRecord.java @@ -41,7 +41,7 @@ public class UnimplementedHemfRecord implements HemfRecord { public long init(LittleEndianInputStream leis, long recordId, long recordSize) throws IOException { this.recordId = recordId; long skipped = IOUtils.skipFully(leis, recordSize); - if (skipped < 0) { + if (skipped < recordSize) { throw new IOException("End of stream reached before record read"); } return skipped; diff --git a/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java b/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java index 495eb465e3..0cc33fdaa2 100644 --- a/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java +++ b/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java @@ -164,8 +164,6 @@ public class HemfExtractorTest { assertEquals(expectedParts.size(), foundExpected); } - - @Test(expected = RecordFormatException.class) public void testInfiniteLoopOnFile() throws Exception { InputStream is = null; diff --git a/src/testcases/org/apache/poi/util/TestIOUtils.java b/src/testcases/org/apache/poi/util/TestIOUtils.java index 8026820e79..825e35aa8b 100644 --- a/src/testcases/org/apache/poi/util/TestIOUtils.java +++ b/src/testcases/org/apache/poi/util/TestIOUtils.java @@ -37,16 +37,16 @@ import org.junit.Test; * Class to test IOUtils */ public final class TestIOUtils { + static File TMP = null; - static long SEED = new Random().nextLong(); - static Random RANDOM = new Random(SEED); + static final long LENGTH = new Random().nextInt(10000); @BeforeClass public static void setUp() throws IOException { TMP = File.createTempFile("poi-ioutils-", ""); OutputStream os = new FileOutputStream(TMP); - for (int i = 0; i < RANDOM.nextInt(10000); i++) { - os.write(RANDOM.nextInt((byte)127)); + for (int i = 0; i < LENGTH; i++) { + os.write(0x01); } os.flush(); os.close(); @@ -62,14 +62,14 @@ public final class TestIOUtils { public void testSkipFully() throws IOException { InputStream is = new FileInputStream(TMP); long skipped = IOUtils.skipFully(is, 20000L); - assertEquals("seed: "+SEED, -1L, skipped); + assertEquals("length: "+LENGTH, LENGTH, skipped); } @Test public void testSkipFullyGtIntMax() throws IOException { InputStream is = new FileInputStream(TMP); long skipped = IOUtils.skipFully(is, Integer.MAX_VALUE + 20000L); - assertEquals("seed: "+SEED, -1L, skipped); + assertEquals("length: "+LENGTH, LENGTH, skipped); } @Test @@ -78,7 +78,7 @@ public final class TestIOUtils { InputStream is = new FileInputStream(TMP); IOUtils.copy(is, bos); long skipped = IOUtils.skipFully(new ByteArrayInputStream(bos.toByteArray()), 20000L); - assertEquals("seed: "+SEED, -1L, skipped); + assertEquals("length: "+LENGTH, LENGTH, skipped); } @Test @@ -87,13 +87,31 @@ public final class TestIOUtils { InputStream is = new FileInputStream(TMP); IOUtils.copy(is, bos); long skipped = IOUtils.skipFully(new ByteArrayInputStream(bos.toByteArray()), Integer.MAX_VALUE+ 20000L); - assertEquals("seed: "+SEED, -1L, skipped); + assertEquals("length: "+LENGTH, LENGTH, skipped); + } + + @Test + public void testZeroByte() throws IOException { + long skipped = IOUtils.skipFully((new ByteArrayInputStream(new byte[0])), 100); + assertEquals("zero byte", -1L, skipped); + } + + @Test + public void testSkipZero() throws IOException { + InputStream is = new FileInputStream(TMP); + long skipped = IOUtils.skipFully(is, 0); + assertEquals("zero length", 0, skipped); + } + @Test(expected = IllegalArgumentException.class) + public void testSkipNegative() throws IOException { + InputStream is = new FileInputStream(TMP); + long skipped = IOUtils.skipFully(is, -1); } @Test public void testWonkyInputStream() throws IOException { long skipped = IOUtils.skipFully(new WonkyInputStream(), 10000); - assertEquals("seed: "+SEED, 10000, skipped); + assertEquals("length: "+LENGTH, 10000, skipped); } /**