From befd31663f10a2059e2a1f6c7111955f4312e17b Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 14 Dec 2019 13:10:17 +0000 Subject: [PATCH] Bug 63569: Adjust handling of check for max allocation of byte array git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1871506 13f79535-47bb-0310-9956-ffa450edef68 --- src/java/org/apache/poi/util/IOUtils.java | 51 ++++++++++++---- .../org/apache/poi/util/TestIOUtils.java | 58 +++++++++++++++++++ 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/java/org/apache/poi/util/IOUtils.java b/src/java/org/apache/poi/util/IOUtils.java index 1e2f12eed4..1f5214589e 100644 --- a/src/java/org/apache/poi/util/IOUtils.java +++ b/src/java/org/apache/poi/util/IOUtils.java @@ -42,20 +42,32 @@ public final class IOUtils { * The default buffer size to use for the skip() methods. */ private static final int SKIP_BUFFER_SIZE = 2048; - private static int BYTE_ARRAY_MAX_OVERRIDE = -1; private static byte[] SKIP_BYTE_BUFFER; + /** + * The current set global allocation limit override, + * -1 means limits are applied per record type. + */ + private static int BYTE_ARRAY_MAX_OVERRIDE = -1; + private IOUtils() { // no instances of this class } /** * If this value is set to > 0, {@link #safelyAllocate(long, int)} will ignore the - * maximum record length parameter. This is designed to allow users to bypass - * the hard-coded maximum record lengths if they are willing to accept the risk - * of an OutOfMemoryException. + * maximum record length parameter. + * + * This is designed to allow users to bypass the hard-coded maximum record lengths + * if they are willing to accept the risk of allocating memory up to the size specified. * - * @param maxOverride The number of bytes that should be possible to be allocated in one step. + * It also allows to impose a lower limit than used for very memory constrained systems. + * + * Note: This is an per-allocation limit and does not allow to limit overall sum of allocations! + * + * Use -1 for using the limits specified per record-type. + * + * @param maxOverride The maximum number of bytes that should be possible to be allocated in one step. * @since 4.0.0 */ @SuppressWarnings("unused") @@ -66,7 +78,7 @@ public final class IOUtils { /** * Peeks at the first 8 bytes of the stream. Returns those bytes, but * with the stream unaffected. Requires a stream that supports mark/reset, - * or a PushbackInputStream. If the stream has >0 but <8 bytes, + * or a PushbackInputStream. If the stream has >0 but <8 bytes, * remaining bytes will be zero. * @throws EmptyFileException if the stream is empty */ @@ -74,6 +86,12 @@ public final class IOUtils { return peekFirstNBytes(stream, 8); } + private static void checkByteSizeLimit(int length) { + if(BYTE_ARRAY_MAX_OVERRIDE != -1 && length > BYTE_ARRAY_MAX_OVERRIDE) { + throwRFE(length, BYTE_ARRAY_MAX_OVERRIDE); + } + } + /** * Peeks at the first N bytes of the stream. Returns those bytes, but * with the stream unaffected. Requires a stream that supports mark/reset, @@ -82,6 +100,8 @@ public final class IOUtils { * @throws EmptyFileException if the stream is empty */ public static byte[] peekFirstNBytes(InputStream stream, int limit) throws IOException, EmptyFileException { + checkByteSizeLimit(limit); + stream.mark(limit); ByteArrayOutputStream bos = new ByteArrayOutputStream(limit); copy(new BoundedInputStream(stream, limit), bos); @@ -149,7 +169,9 @@ public final class IOUtils { if (length > (long)Integer.MAX_VALUE) { throw new RecordFormatException("Can't allocate an array > "+Integer.MAX_VALUE); } - checkLength(length, maxLength); + if ((length != Integer.MAX_VALUE) || (maxLength != Integer.MAX_VALUE)) { + checkLength(length, maxLength); + } final int len = Math.min((int)length, maxLength); ByteArrayOutputStream baos = new ByteArrayOutputStream(len == Integer.MAX_VALUE ? 4096 : len); @@ -162,6 +184,8 @@ public final class IOUtils { if (readBytes > 0) { baos.write(buffer, 0, readBytes); } + + checkByteSizeLimit(readBytes); } while (totalBytes < len && readBytes > -1); if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { @@ -197,6 +221,7 @@ public final class IOUtils { return buffer.array(); } + checkByteSizeLimit(length); byte[] data = new byte[length]; buffer.get(data); return data; @@ -212,12 +237,12 @@ public final class IOUtils { /** *

Same as the normal {@link InputStream#read(byte[], int, int)}, but tries to ensure * that the entire len number of bytes is read.

- * + * *

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.

- * + * * @param in the stream from which the data is read. * @param b the buffer into which the data is read. * @param off the start offset in array b at which the data is written. @@ -483,12 +508,11 @@ public final class IOUtils { } return sum.getValue(); } - - + /** * Quietly (no exceptions) close Closable resource. In case of error it will * be printed to {@link IOUtils} class logger. - * + * * @param closeable * resource to close */ @@ -570,6 +594,9 @@ public final class IOUtils { public static byte[] safelyAllocate(long length, int maxLength) { safelyAllocateCheck(length, maxLength); + + checkByteSizeLimit((int)length); + return new byte[(int)length]; } diff --git a/src/testcases/org/apache/poi/util/TestIOUtils.java b/src/testcases/org/apache/poi/util/TestIOUtils.java index 1445cb9978..73faa9a57e 100644 --- a/src/testcases/org/apache/poi/util/TestIOUtils.java +++ b/src/testcases/org/apache/poi/util/TestIOUtils.java @@ -19,7 +19,9 @@ package org.apache.poi.util; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -202,6 +204,62 @@ public final class TestIOUtils { assertEquals("length: "+LENGTH, 10000, skipped); } + @Test + public void testSetMaxOverride() throws IOException { + ByteArrayInputStream stream = new ByteArrayInputStream("abc".getBytes(StandardCharsets.UTF_8)); + byte[] bytes = IOUtils.toByteArray(stream); + assertNotNull(bytes); + assertEquals("abc", new String(bytes, StandardCharsets.UTF_8)); + } + + @Test + public void testSetMaxOverrideLimit() throws IOException { + IOUtils.setByteArrayMaxOverride(30 * 1024 * 1024); + try { + ByteArrayInputStream stream = new ByteArrayInputStream("abc".getBytes(StandardCharsets.UTF_8)); + byte[] bytes = IOUtils.toByteArray(stream); + assertNotNull(bytes); + assertEquals("abc", new String(bytes, StandardCharsets.UTF_8)); + } finally { + IOUtils.setByteArrayMaxOverride(-1); + } + } + + @Test + public void testSetMaxOverrideOverLimit() throws IOException { + IOUtils.setByteArrayMaxOverride(2); + try { + ByteArrayInputStream stream = new ByteArrayInputStream("abc".getBytes(StandardCharsets.UTF_8)); + try { + IOUtils.toByteArray(stream); + fail("Should have caught an Exception here"); + } catch (RecordFormatException e) { + // expected + } + } finally { + IOUtils.setByteArrayMaxOverride(-1); + } + } + + @Test + public void testSafelyAllocate() { + byte[] bytes = IOUtils.safelyAllocate(30, 200); + assertNotNull(bytes); + assertEquals(30, bytes.length); + } + + @Test + public void testSafelyAllocateLimit() { + IOUtils.setByteArrayMaxOverride(40); + try { + byte[] bytes = IOUtils.safelyAllocate(30, 200); + assertNotNull(bytes); + assertEquals(30, bytes.length); + } finally { + IOUtils.setByteArrayMaxOverride(-1); + } + } + /** * This returns 0 for the first call to skip and then reads * as requested. This tests that the fallback to read() works. -- 2.39.5