Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a _Private_IonConstants.ARRAY_MAXIMUM_SIZE #708

Merged
merged 5 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading