Skip to content

Commit

Permalink
bug 61294 -- cleaned up based on PJ Fanning's code review. Went with …
Browse files Browse the repository at this point in the history 
…a copy/paste from commons-io.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1801952 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@tballison
tballison committed Jul 14, 2017
1 parent a91c041 commit c7db66a
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 62 deletions.
107 changes: 61 additions & 46 deletions src/java/org/apache/poi/util/IOUtils.java
View file
Expand Up @@ -36,6 +36,12 @@ Licensed to the Apache Software Foundation (ASF) under one or more
public final class IOUtils {
private static final POILogger logger = POILogFactory.getLogger( IOUtils.class );

/**
* The default buffer size to use for the skip() methods.
*/
private static final int SKIP_BUFFER_SIZE = 2048;
private static byte[] SKIP_BYTE_BUFFER;

private IOUtils() {
// no instances of this class
}
Expand Down Expand Up @@ -360,57 +366,66 @@ public static void closeQuietly( final Closeable closeable ) {
}
}


/**
* Skips bytes from a stream. Returns -1L if len > available() or if EOF was hit before
* the end of the stream.
* Skips bytes from an input byte stream.
* This implementation guarantees that it will read as many bytes
* as possible before giving up; this may not always be the case for
* skip() implementations in subclasses of {@link InputStream}.
* <p>
* 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.
* </p>
* <p>
* This mimics POI's {@link #readFully(InputStream, byte[])}.
* If the end of file is reached before any bytes are read, returns <tt>-1</tt>. 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 <tt>len</tt>
* bytes have been read, will return <tt>len</tt> bytes.</p>
* </p>
* <p>
* Copied nearly verbatim from commons-io 41a3e9c
* </p>
*
* @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;
}

}
23 changes: 19 additions & 4 deletions src/scratchpad/src/org/apache/poi/hemf/record/HemfCommentRecord.java
View file
Expand Up @@ -89,8 +89,15 @@ private byte[] readToByteArray(byte[] initialBytes, LittleEndianInputStream leis
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;
}
Expand All @@ -103,8 +110,16 @@ private byte[] readToByteArray(LittleEndianInputStream leis, long dataSize, long
}

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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/scratchpad/src/org/apache/poi/hemf/record/UnimplementedHemfRecord.java
View file
Expand Up @@ -41,7 +41,7 @@ public HemfRecordType getRecordType() {
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;
Expand Down
2 changes: 0 additions & 2 deletions src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java
View file
Expand Up @@ -164,8 +164,6 @@ public void testWindowsText() throws Exception {
assertEquals(expectedParts.size(), foundExpected);
}



@Test(expected = RecordFormatException.class)
public void testInfiniteLoopOnFile() throws Exception {
InputStream is = null;
Expand Down
36 changes: 27 additions & 9 deletions src/testcases/org/apache/poi/util/TestIOUtils.java
View file
Expand Up @@ -37,16 +37,16 @@ Licensed to the Apache Software Foundation (ASF) under one or more
* 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();
Expand All @@ -62,14 +62,14 @@ public static void tearDown() throws IOException {
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
Expand All @@ -78,7 +78,7 @@ public void testSkipFullyByteArray() throws IOException {
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
Expand All @@ -87,13 +87,31 @@ public void testSkipFullyByteArrayGtIntMax() throws IOException {
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);
}

/**
Expand Down

0 comments on commit c7db66a

Please sign in to comment.