Author: tallison
Date: Thu Jul 13 16:20:28 2017
New Revision: 1801844
URL: http://svn.apache.org/viewvc?rev=1801844&view=rev
Log:
bug 61294 -- prevent infinite loop in IOUtils' skipFully.
Added:
poi/trunk/src/testcases/org/apache/poi/util/TestIOUtils.java
- copied, changed from r1801831,
poi/trunk/src/testcases/org/apache/poi/util/TestLittleEndian.java
poi/trunk/test-data/spreadsheet/61294.emf (with props)
Modified:
poi/site/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/util/IOUtils.java
poi/trunk/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java
Modified: poi/site/src/documentation/content/xdocs/status.xml
URL:
http://svn.apache.org/viewvc/poi/site/src/documentation/content/xdocs/status.xml?rev=1801844&r1=1801843&r2=1801844&view=diff
==============================================================================
--- poi/site/src/documentation/content/xdocs/status.xml (original)
+++ poi/site/src/documentation/content/xdocs/status.xml Thu Jul 13 16:20:28 2017
@@ -58,6 +58,7 @@
<release version="3.17-beta2" date="2017-09-??">
<actions>
+ <action dev="PD" type="fix" fixes-bug="61294" module="POI
Overall">Fix bug that allowed IOUtils.skipFully to enter infinite loop</action>
<action dev="PD" type="fix" fixes-bug="61266"
module="POIFS">More helpful exception on unsupported old MS Write WRI
files</action>
<action dev="PD" type="fix" fixes-bug="61243" module="POI
Overall">Refactor and unify toString/toXml in DDF</action>
<action dev="PD" type="fix" fixes-bug="61182"
module="OPC">Invalid signature created for streamed xlsx file</action>
Modified: poi/trunk/src/java/org/apache/poi/util/IOUtils.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/util/IOUtils.java?rev=1801844&r1=1801843&r2=1801844&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/util/IOUtils.java (original)
+++ poi/trunk/src/java/org/apache/poi/util/IOUtils.java Thu Jul 13 16:20:28 2017
@@ -361,7 +361,7 @@ public final class IOUtils {
}
/**
- * Skips bytes from a stream. Returns -1L if EOF was hit before
+ * Skips bytes from a stream. Returns -1L if len > available() or if EOF
was hit before
* the end of the stream.
*
* @param in inputstream
@@ -370,11 +370,22 @@ public final class IOUtils {
* @throws IOException on IOException
*/
public static long skipFully(InputStream in, long len) throws IOException {
- int total = 0;
+ 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) {
@@ -382,4 +393,24 @@ public final class IOUtils {
}
}
}
+
+ //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;
+ }
+ }
+ }
}
Modified:
poi/trunk/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java?rev=1801844&r1=1801843&r2=1801844&view=diff
==============================================================================
---
poi/trunk/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java
(original)
+++
poi/trunk/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java
Thu Jul 13 16:20:28 2017
@@ -22,6 +22,8 @@ import static org.apache.poi.POITestCase
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.util.HashSet;
import java.util.Set;
@@ -34,6 +36,8 @@ import org.apache.poi.hemf.record.HemfHe
import org.apache.poi.hemf.record.HemfRecord;
import org.apache.poi.hemf.record.HemfRecordType;
import org.apache.poi.hemf.record.HemfText;
+import org.apache.poi.util.IOUtils;
+import org.apache.poi.util.RecordFormatException;
import org.junit.Test;
public class HemfExtractorTest {
@@ -160,8 +164,37 @@ public class HemfExtractorTest {
assertEquals(expectedParts.size(), foundExpected);
}
- /*
+
+
+ @Test(expected = RecordFormatException.class)
+ public void testInfiniteLoopOnFile() throws Exception {
+ InputStream is = null;
+ try {
+ is =
POIDataSamples.getSpreadSheetInstance().openResourceAsStream("61294.emf");
+
+ HemfExtractor ex = new HemfExtractor(is);
+ for (HemfRecord record : ex) {
+
+ }
+ } finally {
+ IOUtils.closeQuietly(is);
+ }
+ }
+
+ @Test(expected = RecordFormatException.class)
+ public void testInfiniteLoopOnByteArray() throws Exception {
+ InputStream is =
POIDataSamples.getSpreadSheetInstance().openResourceAsStream("61294.emf");
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ IOUtils.copy(is, bos);
+ is.close();
+
+ HemfExtractor ex = new HemfExtractor(new
ByteArrayInputStream(bos.toByteArray()));
+ for (HemfRecord record : ex) {
+
+ }
+ }
+
+ /*
govdocs1 064213.doc-0.emf contains an example of extextouta
*/
-
}
\ No newline at end of file
Copied: poi/trunk/src/testcases/org/apache/poi/util/TestIOUtils.java (from
r1801831, poi/trunk/src/testcases/org/apache/poi/util/TestLittleEndian.java)
URL:
http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/util/TestIOUtils.java?p2=poi/trunk/src/testcases/org/apache/poi/util/TestIOUtils.java&p1=poi/trunk/src/testcases/org/apache/poi/util/TestLittleEndian.java&r1=1801831&r2=1801844&rev=1801844&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/util/TestLittleEndian.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/util/TestIOUtils.java Thu Jul 13
16:20:28 2017
@@ -18,347 +18,119 @@
package org.apache.poi.util;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Random;
-import org.apache.poi.util.LittleEndian.BufferUnderrunException;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
import org.junit.Test;
/**
- * Class to test LittleEndian functionality
+ * Class to test IOUtils
*/
-public final class TestLittleEndian {
+public final class TestIOUtils {
+ static File TMP = null;
+ static long SEED = new Random().nextLong();
+ static Random RANDOM = new Random(SEED);
- /**
- * test the getShort() method
- */
- @Test
- public void testGetShort() {
- byte[] testdata = new byte[ LittleEndianConsts.SHORT_SIZE + 1 ];
-
- testdata[0] = 0x01;
- testdata[1] = (byte) 0xFF;
- testdata[2] = 0x02;
- short expected[] = new short[2];
-
- expected[0] = ( short ) 0xFF01;
- expected[1] = 0x02FF;
- assertEquals(expected[0], LittleEndian.getShort(testdata));
- assertEquals(expected[1], LittleEndian.getShort(testdata, 1));
- }
-
- @Test
- public void testGetUShort() {
- byte[] testdata = {
- (byte) 0x01,
- (byte) 0xFF,
- (byte) 0x02,
- };
- byte[] testdata2 = {
- (byte) 0x0D,
- (byte) 0x93,
- (byte) 0xFF,
- };
-
- int expected0 = 0xFF01;
- int expected1 = 0x02FF;
- int expected2 = 0x930D;
- int expected3 = 0xFF93;
- assertEquals(expected0, LittleEndian.getUShort(testdata));
- assertEquals(expected1, LittleEndian.getUShort(testdata, 1));
- assertEquals(expected2, LittleEndian.getUShort(testdata2));
- assertEquals(expected3, LittleEndian.getUShort(testdata2, 1));
-
- byte[] testdata3 = new byte[ LittleEndianConsts.SHORT_SIZE + 1 ];
- LittleEndian.putUShort(testdata3, 0, expected2);
- LittleEndian.putUShort(testdata3, 1, expected3);
- assertEquals(testdata3[0], 0x0D);
- assertEquals(testdata3[1], (byte)0x93);
- assertEquals(testdata3[2], (byte)0xFF);
- assertEquals(expected2, LittleEndian.getUShort(testdata3));
- assertEquals(expected3, LittleEndian.getUShort(testdata3, 1));
-
- }
-
- private static final byte[] _double_array =
- {
- 56, 50, -113, -4, -63, -64, -13, 63, 76, -32, -42, -35, 60, -43, 3, 64
- };
- /** 0x7ff8000000000000 encoded in little endian order */
- private static final byte[] _nan_double_array =
HexRead.readFromString("00 00 00 00 00 00 F8 7F");
- private static final double[] _doubles =
- {
- 1.23456, 2.47912, Double.NaN
- };
-
- /**
- * test the getDouble() method
- */
- @Test
- public void testGetDouble() {
- assertEquals(_doubles[0], LittleEndian.getDouble(_double_array, 0),
0.000001 );
- assertEquals(_doubles[1], LittleEndian.getDouble( _double_array,
LittleEndianConsts.DOUBLE_SIZE), 0.000001);
- assertTrue(Double.isNaN(LittleEndian.getDouble(_nan_double_array, 0)));
-
- double nan = LittleEndian.getDouble(_nan_double_array, 0);
- byte[] data = new byte[8];
- LittleEndian.putDouble(data, 0, nan);
- for ( int i = 0; i < data.length; i++ ) {
- assertEquals(data[i], _nan_double_array[i]);
+ @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));
}
- }
+ os.flush();
+ os.close();
- /**
- * test the getInt() method
- */
- @Test
- public void testGetInt() {
- // reading 4 byte data from a 5 byte buffer
- byte[] testdata = {
- (byte) 0x01,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0x02,
- };
-
- assertEquals(0xFFFFFF01, LittleEndian.getInt(testdata));
- assertEquals(0x02FFFFFF, LittleEndian.getInt(testdata, 1));
}
- /**
- * test the getLong method
- */
- @Test
- public void testGetLong() {
-
- // reading 8 byte values from a 9 byte buffer
- byte[] testdata = {
- (byte) 0x01,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0x02,
- };
-
- assertEquals(0xFFFFFFFFFFFFFF01L, LittleEndian.getLong(testdata, 0));
- assertEquals(0x02FFFFFFFFFFFFFFL, LittleEndian.getLong(testdata, 1));
+ @AfterClass
+ public static void tearDown() throws IOException {
+ TMP.delete();
}
- /**
- * test the PutShort method
- */
@Test
- public void testPutShort() {
- byte[] expected = new byte[ LittleEndianConsts.SHORT_SIZE + 1 ];
-
- expected[0] = 0x01;
- expected[1] = (byte) 0xFF;
- expected[2] = 0x02;
- byte[] received = new byte[ LittleEndianConsts.SHORT_SIZE + 1 ];
- short testdata[] = new short[2];
-
- testdata[0] = ( short ) 0xFF01;
- testdata[1] = 0x02FF;
- LittleEndian.putShort(received, 0, testdata[0]);
- assertTrue(compareByteArrays(received, expected, 0,
LittleEndianConsts.SHORT_SIZE));
- LittleEndian.putShort(received, 1, testdata[1]);
- assertTrue(compareByteArrays(received, expected, 1,
LittleEndianConsts.SHORT_SIZE));
+ public void testSkipFully() throws IOException {
+ InputStream is = new FileInputStream(TMP);
+ long skipped = IOUtils.skipFully(is, 20000L);
+ assertEquals("seed: "+SEED, -1L, skipped);
}
- /**
- * test the putInt method
- */
@Test
- public void testPutInt() {
- // writing 4 byte data to a 5 byte buffer
- byte[] expected = {
- (byte) 0x01,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0x02,
- };
- byte[] received = new byte[ LittleEndianConsts.INT_SIZE + 1 ];
-
- LittleEndian.putInt(received, 0, 0xFFFFFF01);
- assertTrue(compareByteArrays(received, expected, 0,
LittleEndianConsts.INT_SIZE));
- LittleEndian.putInt(received, 1, 0x02FFFFFF);
- assertTrue(compareByteArrays(received, expected, 1,
LittleEndianConsts.INT_SIZE));
+ public void testSkipFullyGtIntMax() throws IOException {
+ InputStream is = new FileInputStream(TMP);
+ long skipped = IOUtils.skipFully(is, Integer.MAX_VALUE + 20000L);
+ assertEquals("seed: "+SEED, -1L, skipped);
}
- /**
- * test the putDouble methods
- */
@Test
- public void testPutDouble() {
- byte[] received = new byte[ LittleEndianConsts.DOUBLE_SIZE + 1 ];
-
- LittleEndian.putDouble(received, 0, _doubles[0]);
- assertTrue(compareByteArrays(received, _double_array, 0,
LittleEndianConsts.DOUBLE_SIZE));
- LittleEndian.putDouble(received, 1, _doubles[1]);
- byte[] expected = new byte[ LittleEndianConsts.DOUBLE_SIZE + 1 ];
-
- System.arraycopy(_double_array, LittleEndianConsts.DOUBLE_SIZE,
expected,
- 1, LittleEndianConsts.DOUBLE_SIZE);
- assertTrue(compareByteArrays(received, expected, 1,
LittleEndianConsts.DOUBLE_SIZE));
+ public void testSkipFullyByteArray() throws IOException {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ InputStream is = new FileInputStream(TMP);
+ IOUtils.copy(is, bos);
+ long skipped = IOUtils.skipFully(new
ByteArrayInputStream(bos.toByteArray()), 20000L);
+ assertEquals("seed: "+SEED, -1L, skipped);
}
- /**
- * test the putLong method
- */
@Test
- public void testPutLong() {
- // writing 8 byte values to a 9 byte buffer
- byte[] expected = {
- (byte) 0x01,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0xFF,
- (byte) 0x02,
- };
- byte[] received = new byte[ LittleEndianConsts.LONG_SIZE + 1 ];
-
- long testdata0 = 0xFFFFFFFFFFFFFF01L;
- long testdata1 = 0x02FFFFFFFFFFFFFFL;
- LittleEndian.putLong(received, 0, testdata0);
- assertTrue(compareByteArrays(received, expected, 0,
LittleEndianConsts.LONG_SIZE));
- LittleEndian.putLong(received, 1, testdata1);
- assertTrue(compareByteArrays(received, expected, 1,
LittleEndianConsts.LONG_SIZE));
+ public void testSkipFullyByteArrayGtIntMax() throws IOException {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ 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);
}
- private static byte[] _good_array = {
- 0x01, 0x02, 0x01, 0x02,
- 0x01, 0x02, 0x01, 0x02,
- 0x01, 0x02, 0x01, 0x02,
- 0x01, 0x02, 0x01, 0x02,
- };
- private static byte[] _bad_array = {
- 0x01
- };
-
- /**
- * test the readShort method
- */
@Test
- public void testReadShort() throws IOException {
- short expected_value = 0x0201;
- InputStream stream = new ByteArrayInputStream(_good_array);
- int count = 0;
-
- while (stream.available() > 0) {
- short value = LittleEndian.readShort(stream);
- assertEquals(value, expected_value);
- count++;
- }
- assertEquals(count,
- _good_array.length / LittleEndianConsts.SHORT_SIZE);
- stream = new ByteArrayInputStream(_bad_array);
- try {
- LittleEndian.readShort(stream);
- fail("Should have caught BufferUnderrunException");
- } catch (BufferUnderrunException ignored) {
- // as expected
- }
+ public void testWonkyInputStream() throws IOException {
+ long skipped = IOUtils.skipFully(new WonkyInputStream(), 10000);
+ assertEquals("seed: "+SEED, 10000, skipped);
}
/**
- * test the readInt method
+ * This returns 0 for the first call to skip and then reads
+ * as requested. This tests that the fallback to read() works.
*/
- @Test
- public void testReadInt() throws IOException {
- int expected_value = 0x02010201;
- InputStream stream = new ByteArrayInputStream(_good_array);
- int count = 0;
-
- while (stream.available() > 0) {
- int value = LittleEndian.readInt(stream);
- assertEquals(value, expected_value);
- count++;
- }
- assertEquals(count, _good_array.length / LittleEndianConsts.INT_SIZE);
- stream = new ByteArrayInputStream(_bad_array);
- try {
- LittleEndian.readInt(stream);
- fail("Should have caught BufferUnderrunException");
- } catch (BufferUnderrunException ignored) {
+ private static class WonkyInputStream extends InputStream {
+ int skipCalled = 0;
+ int readCalled = 0;
- // as expected
+ @Override
+ public int read() throws IOException {
+ readCalled++;
+ return 0;
}
- }
- /**
- * test the readLong method
- */
- @Test
- public void testReadLong() throws IOException {
- long expected_value = 0x0201020102010201L;
- InputStream stream = new ByteArrayInputStream(_good_array);
- int count = 0;
-
- while (stream.available() > 0) {
- long value = LittleEndian.readLong(stream);
- assertEquals(value, expected_value);
- count++;
- }
- assertEquals(count,
- _good_array.length / LittleEndianConsts.LONG_SIZE);
- stream = new ByteArrayInputStream(_bad_array);
- try {
- LittleEndian.readLong(stream);
- fail("Should have caught BufferUnderrunException");
- } catch (BufferUnderrunException ignored) {
- // as expected
+ @Override
+ public int read(byte[] arr, int offset, int len) throws IOException {
+ readCalled++;
+ return len;
}
- }
-
-// public void testReadFromStream() throws IOException {
-// int actual;
-// actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[]
{ 5, -128, }));
-// assertEquals(32773, actual);
-//
-// actual = LittleEndian.readUShort(new ByteArrayInputStream(new byte[]
{ 1, 2, 3, 4, }));
-// assertEquals(513, actual);
-//
-// try {
-// LittleEndian.readInt(new ByteArrayInputStream(new byte[] { 1, 2,
3, }));
-// fail("Should have caught BufferUnderrunException");
-// } catch (BufferUnderrunException ignored) {
-// // as expected
-// }
-// }
- @Test
- public void testUnsignedByteToInt() {
- assertEquals(255, LittleEndian.ubyteToInt((byte)255));
- }
-
- private static boolean compareByteArrays(byte [] received, byte []
expected,
- int offset, int size) {
-
- for (int j = offset; j < offset + size; j++) {
- if (received[j] != expected[j]) {
- System.out.println("difference at index " + j);
- return false;
+ @Override
+ public long skip(long len) throws IOException {
+ skipCalled++;
+ if (skipCalled == 1) {
+ return 0;
+ } else if (skipCalled > 100) {
+ return len;
+ } else {
+ return 100;
}
}
- return true;
- }
- @Test
- public void testUnsignedShort() {
- assertEquals(0xffff, LittleEndian.getUShort(new byte[] { (byte)0xff,
(byte)0xff }, 0));
+ @Override
+ public int available() {
+ return 100000;
+ }
}
}
Added: poi/trunk/test-data/spreadsheet/61294.emf
URL:
http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/61294.emf?rev=1801844&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/61294.emf
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
