Skip to content

Commit

Permalink
Add a _Private_IonConstants.ARRAY_MAXIMUM_SIZE (#708)
Browse files Browse the repository at this point in the history
* Add a _Private_IonConstants.ARRAY_MAXIMUM_SIZE

* Use the new constant where appropriate

Arrays in Java must be indexed by integers (JLS 10.4), but the true maximum
array size may be less than `Integer.MAX_VALUE`. True maximum array size is a
JVM implementation detail, and varies by type and by JVM.
We have this pinned at `Integer.MAX_VALUE - 8` because that's a fairly common
value in the JDK itself, in classes such as `ConcurrentHashMap`, `InputStream`,
`Hashtable`, `ByteArrayChannel`, etc.

In testing against a variety of JVMs and types the smallest maximum size I've
seen is `Integer.MAX_VALUE - 2`, and the smallest maximum size I've seen
reported is `Integer.MAX_VALUE - 6`

* Add test coverage for maximum buffer size

* Add more test coverage for maximum buffer size

* Ensure that a modeled exception is thrown
  • Loading branch information
jobarr-amzn authored Jan 30, 2024
1 parent 3c1b6b1 commit 88035b9
Show file tree
Hide file tree
Showing 15 changed files with 119 additions and 24 deletions.
14 changes: 8 additions & 6 deletions src/main/java/com/amazon/ion/BufferConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

package com.amazon.ion;

import com.amazon.ion.impl._Private_IonConstants;

/**
* Provides logic common to all BufferConfiguration implementations.
* @param <Configuration> the type of the concrete subclass of this BufferConfiguration.
Expand Down Expand Up @@ -60,7 +62,7 @@ public static abstract class Builder<
/**
* The maximum number of bytes that will be buffered.
*/
private int maximumBufferSize = Integer.MAX_VALUE;
private int maximumBufferSize = _Private_IonConstants.ARRAY_MAXIMUM_SIZE;

/**
* The handler that will be notified when oversized values are encountered.
Expand Down Expand Up @@ -133,7 +135,7 @@ public final DataHandler getDataHandler() {
* Set the maximum size of the buffer. For binary Ion, the minimum value is 5 because all valid binary Ion data
* begins with a 4-byte Ion version marker and the smallest value is 1 byte. For text Ion, the minimum value is
* 2 because the smallest text Ion value is 1 byte and the smallest delimiter is 1 byte.
* Default: Integer.MAX_VALUE.
* Default: Near to the maximum size of an array.
*
* @param maximumBufferSizeInBytes the value.
* @return this builder.
Expand Down Expand Up @@ -214,7 +216,7 @@ protected BufferConfiguration(Builder<Configuration, ?> builder) {
));
}
if (builder.getOversizedValueHandler() == null) {
requireUnlimitedBufferSize();
requireMaximumBufferSize();
oversizedValueHandler = builder.getThrowingOversizedValueHandler();
} else {
oversizedValueHandler = builder.getOversizedValueHandler();
Expand All @@ -229,10 +231,10 @@ protected BufferConfiguration(Builder<Configuration, ?> builder) {
/**
* Requires that the maximum buffer size not be limited.
*/
protected void requireUnlimitedBufferSize() {
if (maximumBufferSize < Integer.MAX_VALUE) {
protected void requireMaximumBufferSize() {
if (maximumBufferSize < _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
throw new IllegalArgumentException(
"Must specify an OversizedValueHandler when a maximum buffer size is specified."
"Must specify an OversizedValueHandler when a custom maximum buffer size is specified."
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/ion/IonBufferConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public IonBufferConfiguration build() {
private IonBufferConfiguration(Builder builder) {
super(builder);
if (builder.getOversizedSymbolTableHandler() == null) {
requireUnlimitedBufferSize();
requireMaximumBufferSize();
oversizedSymbolTableHandler = builder.getThrowingOversizedSymbolTableHandler();
} else {
oversizedSymbolTableHandler = builder.getOversizedSymbolTableHandler();
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/amazon/ion/apps/BaseApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.amazon.ion.IonReader;
import com.amazon.ion.IonSystem;
import com.amazon.ion.SymbolTable;
import com.amazon.ion.impl._Private_IonConstants;
import com.amazon.ion.system.IonSystemBuilder;
import com.amazon.ion.system.SimpleCatalog;
import java.io.BufferedInputStream;
Expand Down Expand Up @@ -65,7 +66,7 @@ protected static byte[] loadAsByteArray(File file)
throws FileNotFoundException, IOException
{
long len = file.length();
if (len < 0 || len > Integer.MAX_VALUE)
if (len < 0 || len > Integer.MAX_VALUE - 8)
{
throw new IllegalArgumentException("File too long: " + file);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/ion/impl/BlockedBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,7 @@ private final void fail_on_version_change() throws IOException
@Override
public final long skip(long n) throws IOException
{
if (n < 0 || n > Integer.MAX_VALUE) throw new IllegalArgumentException("we only handle buffer less than "+Integer.MAX_VALUE+" bytes in length");
if (n < 0 || n > _Private_IonConstants.ARRAY_MAXIMUM_SIZE) throw new IllegalArgumentException("we only handle buffer less than "+_Private_IonConstants.ARRAY_MAXIMUM_SIZE+" bytes in length");
if (_buf == null) throw new IOException("stream is closed");
fail_on_version_change();
if (_pos >= _buf.size()) return -1;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ private static IonBufferConfiguration getFixedSizeConfigurationFor(
configuration.getInitialBufferSize(),
configuration.getMaximumBufferSize()
);
registerOversizedValueHandler(configuration.getOversizedValueHandler());
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ private int load_lob_contents() throws IOException
}
if (_lob_loaded == LOB_STATE.READ) {
long raw_size = _current_value_save_point.length();
if (raw_size < 0 || raw_size > Integer.MAX_VALUE) {
if (raw_size < 0 || raw_size > _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
load_lob_length_overflow_error(raw_size);
}
_lob_bytes = new byte[(int)raw_size];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void bytesConsolidatedToStartOfBuffer(int leftShiftAmount) {
* such that growth is expected to occur rarely, if ever.
*/
public ResizingPipedInputStream(final int initialBufferSize) {
this(initialBufferSize, Integer.MAX_VALUE, false);
this(initialBufferSize, _Private_IonConstants.ARRAY_MAXIMUM_SIZE, false);
}

/**
Expand All @@ -152,6 +152,9 @@ public ResizingPipedInputStream(final int initialBufferSize) {
if (initialBufferSize < 1) {
throw new IllegalArgumentException("Initial buffer size must be at least 1.");
}
if (maximumBufferSize > _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
throw new IllegalArgumentException("Initial buffer size must be at most " + _Private_IonConstants.ARRAY_MAXIMUM_SIZE + ".");
}
if (maximumBufferSize < initialBufferSize) {
throw new IllegalArgumentException("Maximum buffer size cannot be less than the initial buffer size.");
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/amazon/ion/impl/UnifiedInputBufferX.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ protected static final char[] chars_make_char_array(CharSequence chars,
private UnifiedInputBufferX(int initialPageSize) {
if (initialPageSize < 0) {
throw new IllegalArgumentException("page size must be > 0");
} else if (initialPageSize > _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
throw new IllegalArgumentException("page size must be < " + _Private_IonConstants.ARRAY_MAXIMUM_SIZE);
}
_page_size = initialPageSize;
_buffers = new UnifiedDataPageX[10];
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/amazon/ion/impl/UnifiedInputStreamX.java
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ private static class FromCharStream extends UnifiedInputStreamX
_is_byte_data = false;
_is_stream = true;
_reader = reader;
// If this page size ever becomes configurable watch out for _Private_IonConstants.ARRAY_MAXIMUM_SIZE
_buffer = UnifiedInputBufferX.makePageBuffer(UnifiedInputBufferX.BufferType.CHARS, DEFAULT_PAGE_SIZE);
super.init();
_limit = refill();
Expand Down Expand Up @@ -601,6 +602,7 @@ private static class FromByteStream extends UnifiedInputStreamX
_is_byte_data = true;
_is_stream = true;
_stream = stream;
// If this page size ever becomes configurable watch out for _Private_IonConstants.ARRAY_MAXIMUM_SIZE
_buffer = UnifiedInputBufferX.makePageBuffer(UnifiedInputBufferX.BufferType.BYTES, DEFAULT_PAGE_SIZE);
super.init();
_limit = refill();
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/amazon/ion/impl/_Private_IonConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
*/
public final class _Private_IonConstants
{
// Arrays in Java must be indexed by integers (JLS 10.4), but the true maximum array size may be less than
// Integer.MAX_VALUE. True maximum array size is a JVM implementation detail, and varies by type and by JVM.
// We have this pinned at Integer.MAX_VALUE - 8 because that's a fairly common value in the JDK itself, in
// classes such as ConcurrentHashMap, InputStream, Hashtable, ByteArrayChannel, etc.
// In testing against a variety of JVMs and types the smallest maximum size I've seen is Integer.MAX_VALUE - 2,
// and the smallest maximum size I've seen reported is Integer.MAX_VALUE - 6.
public static final int ARRAY_MAXIMUM_SIZE = Integer.MAX_VALUE - 8;

private _Private_IonConstants() { }


Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/ion/impl/_Private_Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ public static byte[] loadFileBytes(File file)
throws IOException
{
long len = file.length();
if (len < 0 || len > Integer.MAX_VALUE) {
if (len < 0 || len > _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
throw new IllegalArgumentException("File too long: " + file);
}

Expand Down
30 changes: 30 additions & 0 deletions src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package com.amazon.ion.impl;

import com.amazon.ion.IonBufferConfiguration;
import com.amazon.ion.IonCursor;
import com.amazon.ion.IonException;
import org.junit.jupiter.api.Test;
Expand All @@ -27,6 +28,8 @@
import static com.amazon.ion.impl.IonCursorTestUtilities.endStream;
import static com.amazon.ion.impl.IonCursorTestUtilities.scalar;
import static com.amazon.ion.impl.IonCursorTestUtilities.startContainer;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

Expand Down Expand Up @@ -397,4 +400,31 @@ public void expectMissingAnnotationWrapperAnnotationLengthToFailCleanly() {
assertThrows(IonException.class, cursor::nextValue);
cursor.close();
}

@Test
public void expectValueLargerThanIntMaxToFailCleanly() {
int[] data = new int[] {
0xE0, 0x01, 0x00, 0xEA,
0x2E, // Integer with VarUInt length, because that's clearly a reasonable thing to find
0x07, 0x7f, 0x7f, 0x7f, 0xf9 // VarUInt length 2147483647 (Integer.MAX_LENGTH)
};
ByteArrayInputStream in = new ByteArrayInputStream(bytes(data));

// We need a custom initial buffer size so that the cursor doesn't know there are fewer bytes remaining
// than the value header promises
IonBufferConfiguration config = IonBufferConfiguration.Builder.standard()
.withInitialBufferSize(8)
.build();

IonCursorBinary cursor = new IonCursorBinary(config, in, null, 0, 0);

cursor.nextValue(); // Position cursor on unreal oversized value

// Try to get all the value bytes, and fail because arrays can't be that large
IonException ie = assertThrows(IonException.class, cursor::fillValue);
assertThat(ie.getMessage(),
containsString("An oversized value was found even though no maximum size was configured."));

cursor.close();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@
import java.util.zip.GZIPOutputStream;

import static com.amazon.ion.BitUtils.bytes;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.*;
import static org.junit.runners.Parameterized.Parameter;
import static org.junit.runners.Parameterized.Parameters;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;

@SuppressWarnings("resource")
@RunWith(Parameterized.class)
public class ResizingPipedInputStreamTest {

Expand Down Expand Up @@ -94,7 +93,7 @@ public static Iterable<Object[]> bufferSizes() {

@Before
public void setup() {
input = new ResizingPipedInputStream(bufferSize, Integer.MAX_VALUE, useBoundary);
input = new ResizingPipedInputStream(bufferSize, _Private_IonConstants.ARRAY_MAXIMUM_SIZE, useBoundary);
knownCapacity = input.capacity();
}

Expand Down Expand Up @@ -469,16 +468,20 @@ public void truncate() throws Exception {
assertEquals(0, input.available());
}

@Test(expected = IllegalArgumentException.class)
@Test
public void errorsOnInvalidInitialBufferSize() {
new ResizingPipedInputStream(0);
assertThrows(IllegalArgumentException.class, () -> new ResizingPipedInputStream(0));
}

@Test(expected = IllegalArgumentException.class)
public void errorsOnInvalidMaximumBufferSize() {
new ResizingPipedInputStream(10, 9, useBoundary);
@Test
public void errorsOnMaximumBufferSizeSmallerThanInitialSize() {
assertThrows(IllegalArgumentException.class, () -> new ResizingPipedInputStream(10, 9, useBoundary));
}

@Test
public void errorsOnInvalidMaximumBufferSize() {
assertThrows(IllegalArgumentException.class, () -> new ResizingPipedInputStream(10, Integer.MAX_VALUE, useBoundary));
}
@Test
public void initialBufferSizeAndCapacity() {
assertEquals(bufferSize, input.getInitialBufferSize());
Expand Down
40 changes: 40 additions & 0 deletions src/test/java/com/amazon/ion/impl/UnifiedInputBufferXTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2007-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file 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 com.amazon.ion.impl;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class UnifiedInputBufferXTest {

@Test
public void ctorThrowsWithIllegalInitialPageSize() {
assertThrows(IllegalArgumentException.class,
() -> new UnifiedInputBufferX.Chars(-1));

assertThrows(IllegalArgumentException.class,
() -> new UnifiedInputBufferX.Chars(Integer.MAX_VALUE));
}

@Test
public void ctorWorksWithLegalInitialPageSize() {
new UnifiedInputBufferX.Chars(0);
new UnifiedInputBufferX.Chars(_Private_IonConstants.ARRAY_MAXIMUM_SIZE);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

package com.amazon.ion.impl;

import org.junit.Assert;
import org.junit.Test;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;

public class UnifiedInputStreamXTest extends Assert {
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class UnifiedInputStreamXTest {

@Test
public void testReadExactlyAvailable() throws Exception {
class TestInputStream extends InputStream {
Expand Down

0 comments on commit 88035b9

Please sign in to comment.