From: Tim Allison Date: Thu, 13 Jul 2017 16:20:28 +0000 (+0000) Subject: bug 61294 -- prevent infinite loop in IOUtils' skipFully. X-Git-Tag: REL_3_17_FINAL~62 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=34cb8609982f26f1d82a8a343d368217aca45def;p=poi.git bug 61294 -- prevent infinite loop in IOUtils' skipFully. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1801844 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/util/IOUtils.java b/src/java/org/apache/poi/util/IOUtils.java index d8938a61db..62001d1e05 100644 --- a/src/java/org/apache/poi/util/IOUtils.java +++ b/src/java/org/apache/poi/util/IOUtils.java @@ -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; + } + } + } } 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 0849e23aba..495eb465e3 100644 --- a/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java +++ b/src/scratchpad/testcases/org/apache/poi/hemf/extractor/HemfExtractorTest.java @@ -22,6 +22,8 @@ import static org.apache.poi.POITestCase.assertContains; 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.HemfHeader; 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 diff --git a/src/testcases/org/apache/poi/util/TestIOUtils.java b/src/testcases/org/apache/poi/util/TestIOUtils.java new file mode 100644 index 0000000000..8026820e79 --- /dev/null +++ b/src/testcases/org/apache/poi/util/TestIOUtils.java @@ -0,0 +1,136 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.util; + +import static org.junit.Assert.assertEquals; + +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.junit.AfterClass; +import org.junit.BeforeClass; +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); + + @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(); + + } + + @AfterClass + public static void tearDown() throws IOException { + TMP.delete(); + } + + @Test + public void testSkipFully() throws IOException { + InputStream is = new FileInputStream(TMP); + long skipped = IOUtils.skipFully(is, 20000L); + assertEquals("seed: "+SEED, -1L, 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); + } + + @Test + 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 + 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); + } + + @Test + public void testWonkyInputStream() throws IOException { + long skipped = IOUtils.skipFully(new WonkyInputStream(), 10000); + assertEquals("seed: "+SEED, 10000, skipped); + } + + /** + * This returns 0 for the first call to skip and then reads + * as requested. This tests that the fallback to read() works. + */ + private static class WonkyInputStream extends InputStream { + int skipCalled = 0; + int readCalled = 0; + + @Override + public int read() throws IOException { + readCalled++; + return 0; + } + + @Override + public int read(byte[] arr, int offset, int len) throws IOException { + readCalled++; + return len; + } + + @Override + public long skip(long len) throws IOException { + skipCalled++; + if (skipCalled == 1) { + return 0; + } else if (skipCalled > 100) { + return len; + } else { + return 100; + } + } + + @Override + public int available() { + return 100000; + } + } +} diff --git a/test-data/spreadsheet/61294.emf b/test-data/spreadsheet/61294.emf new file mode 100644 index 0000000000..886b9e3de1 Binary files /dev/null and b/test-data/spreadsheet/61294.emf differ