Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 24 additions & 2 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,11 @@ private boolean ensureCapacity(long minimumNumberOfBytesRequired) {
}
long shortfall = minimumNumberOfBytesRequired - refillableState.capacity;
if (shortfall > 0) {
int newSize = (int) Math.min(Math.max(refillableState.capacity * 2, nextPowerOfTwo((int) (refillableState.capacity + shortfall))), maximumFreeSpace);
// Grow by doubling rather than allocating the full declared length up front. This prevents a
// small malicious payload from triggering a disproportionately large allocation based on an
// attacker-declared VarUInt length. If the value is legitimately large, refill() will continue
// to grow the buffer as actual data is consumed from the stream.
int newSize = (int) Math.min(Math.max(refillableState.capacity * 2L, refillableState.capacity + 1L), maximumFreeSpace);
byte[] newBuffer = new byte[newSize];
moveBytesToStartOfBuffer(newBuffer, startOffset);
refillableState.capacity = newSize;
Expand Down Expand Up @@ -672,11 +676,29 @@ private void shiftIndicesLeft(int shiftAmount) {
*/
private long refill(long minimumNumberOfBytesRequired) {
int numberOfBytesFilled = -1;
long shortfall;
long shortfall = minimumNumberOfBytesRequired - availableAt(offset);
// Sometimes an InputStream implementation will return fewer than the number of bytes requested even
// if the stream is not at EOF. If this happens and there is still a shortfall, keep requesting bytes
// until either the shortfall is filled or EOF is reached.
do {
// If the buffer is full but more bytes are still required, grow it (by doubling) to make room.
// Growing here, after data has been read, ensures allocation is proportional to the data actually
// present in the stream rather than to an attacker-declared length. Because ensureCapacity() has
// already moved buffered bytes to the start of the buffer (offset == 0), growing only requires
// copying the existing bytes into a larger array; no parser indices need to be shifted.
if (freeSpaceAt(limit) == 0 && (minimumNumberOfBytesRequired - availableAt(offset)) > 0) {
if (refillableState.capacity >= refillableState.maximumBufferSize) {
// Cannot grow further; the value exceeds the maximum buffer size.
refillableState.isSkippingCurrentValue = true;
break;
}
int newSize = (int) Math.min(Math.max(refillableState.capacity * 2L, refillableState.capacity + 1L), refillableState.maximumBufferSize);
byte[] newBuffer = new byte[newSize];
System.arraycopy(buffer, 0, newBuffer, 0, (int) limit);
refillableState.capacity = newSize;
buffer = newBuffer;
byteBuffer = ByteBuffer.wrap(buffer, (int) offset, (int) refillableState.capacity);
}
try {
numberOfBytesFilled = refillableState.inputStream.read(buffer, (int) limit, (int) freeSpaceAt(limit));
} catch (EOFException e) {
Expand Down
59 changes: 59 additions & 0 deletions src/main/java/com/amazon/ion/system/IonReaderBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public abstract class IonReaderBuilder
private boolean isIncrementalReadingEnabled = false;
private IonBufferConfiguration bufferConfiguration = IonBufferConfiguration.DEFAULT;
private List<InputStreamInterceptor> streamInterceptors = null;
private boolean gzipDecompressionEnabled = true;

protected IonReaderBuilder()
{
Expand All @@ -112,6 +113,7 @@ protected IonReaderBuilder(IonReaderBuilder that)
this.isIncrementalReadingEnabled = that.isIncrementalReadingEnabled;
this.bufferConfiguration = that.bufferConfiguration;
this.streamInterceptors = that.streamInterceptors == null ? null : new ArrayList<>(that.streamInterceptors);
this.gzipDecompressionEnabled = that.gzipDecompressionEnabled;
}

/**
Expand Down Expand Up @@ -385,12 +387,69 @@ private static List<InputStreamInterceptor> detectStreamInterceptorsOnClasspath(
* @return an unmodifiable view of the stream interceptors currently configured.
*/
public List<InputStreamInterceptor> getInputStreamInterceptors() {
if (!gzipDecompressionEnabled) {
if (streamInterceptors == null) {
// Filter out GzipStreamInterceptor from the detected list
List<InputStreamInterceptor> filtered = new ArrayList<>();
for (InputStreamInterceptor interceptor : DETECTED_STREAM_INTERCEPTORS) {
if (!(interceptor instanceof GzipStreamInterceptor)) {
filtered.add(interceptor);
}
}
return Collections.unmodifiableList(filtered);
}
// Filter out GzipStreamInterceptor from the custom list
List<InputStreamInterceptor> filtered = new ArrayList<>();
for (InputStreamInterceptor interceptor : streamInterceptors) {
if (!(interceptor instanceof GzipStreamInterceptor)) {
filtered.add(interceptor);
}
}
return Collections.unmodifiableList(filtered);
}
if (streamInterceptors == null) {
return DETECTED_STREAM_INTERCEPTORS;
}
return Collections.unmodifiableList(streamInterceptors);
}

/**
* Declares whether GZIP auto-decompression is enabled when building an {@link IonReader},
* returning a new mutable builder if the current one is immutable.
*
* When enabled (the default), GZIP-compressed Ion data is automatically detected and decompressed.
* When disabled, GZIP-compressed data is not auto-decompressed.
*
* @param enabled true to enable GZIP auto-decompression (default), false to disable.
* @return this builder instance, if mutable; otherwise a mutable copy of this builder.
*/
public IonReaderBuilder withGzipDecompressionEnabled(boolean enabled) {
IonReaderBuilder b = mutable();
b.gzipDecompressionEnabled = enabled;
return b;
}

/**
* Sets whether GZIP auto-decompression is enabled.
*
* @param enabled true to enable GZIP auto-decompression (default), false to disable.
*
* @see #withGzipDecompressionEnabled(boolean)
*
* @throws UnsupportedOperationException if this builder is immutable.
*/
public void setGzipDecompressionEnabled(boolean enabled) {
mutationCheck();
this.gzipDecompressionEnabled = enabled;
}

/**
* @return true if GZIP auto-decompression is enabled (the default).
*/
public boolean isGzipDecompressionEnabled() {
return gzipDecompressionEnabled;
}

/**
* Based on the builder's configuration properties, creates a new IonReader
* instance over the given block of Ion data, detecting whether it's text or
Expand Down
72 changes: 72 additions & 0 deletions src/test/java/com/amazon/ion/impl/IonCursorBinaryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,78 @@ public void expectValueLargerThanMaxArraySizeToFailCleanly() {
cursor.close();
}

/**
* Builds binary Ion for a string value of the given length, with the bytes prefixed by the IVM.
* Uses the VarUInt-length string encoding (type 0x8E) so that lengths greater than 13 are supported.
*/
private static int[] stringValueWithLength(int contentLength) {
// Encode the content length as a VarUInt (single byte for lengths < 128).
if (contentLength >= 128) {
throw new IllegalArgumentException("This helper only supports content lengths < 128.");
}
int[] data = new int[4 + 1 + 1 + contentLength];
data[0] = 0xE0; data[1] = 0x01; data[2] = 0x00; data[3] = 0xEA; // IVM
data[4] = 0x8E; // String with VarUInt length
data[5] = 0x80 | contentLength; // VarUInt length with stop bit
for (int i = 0; i < contentLength; i++) {
data[6 + i] = 'a';
}
return data;
}

/**
* Verifies that a legitimate value larger than the initial buffer (and larger than a single doubling of it)
* is read correctly. This exercises the incremental buffer growth in refill(): the buffer must double
* multiple times as real data is consumed from the stream.
*/
@Test
public void largeValueReadFromStreamGrowsBufferIncrementally() {
int contentLength = 100; // Far larger than the 8-byte initial buffer; requires several doublings.
int[] data = stringValueWithLength(contentLength);
ByteArrayInputStream in = new ByteArrayInputStream(bytes(data));
IonBufferConfiguration config = IonBufferConfiguration.Builder.standard()
.withInitialBufferSize(8)
.build();
IonCursorBinary cursor = new IonCursorBinary(config, in, null, 0, 0);
// After the IVM is consumed, buffered bytes are shifted to the start of the buffer, so the content
// begins at index 2 (after the type ID and VarUInt length bytes) and ends at 2 + contentLength.
assertSequence(
cursor,
fillScalar(2, 2 + contentLength),
endStream()
);
cursor.close();
}

/**
* Verifies that a value declaring a length far larger than the data actually present in the stream does not
* cause a disproportionate allocation. The buffer grows only in proportion to the data actually consumed.
* When a fixed-size stream is exhausted before the declared length is satisfied, the cursor fails cleanly
* with an IonException rather than allocating (or attempting to allocate) the full declared length.
*/
@Test
public void lengthBombFromStreamDoesNotOverAllocate() {
int[] data = new int[]{
0xE0, 0x01, 0x00, 0xEA, // Ion 1.0 IVM
0x8E, // String with VarUInt length
0x07, 0x7f, 0x7f, 0x7f, 0xf9, // VarUInt length ~2 GB
'a', 'b', 'c' // Only a few actual content bytes follow.
};
ByteArrayInputStream in = new ByteArrayInputStream(bytes(data));
IonBufferConfiguration config = IonBufferConfiguration.Builder.standard()
.withInitialBufferSize(8)
.build();
IonCursorBinary cursor = new IonCursorBinary(config, in, null, 0, 0);
// Positioning on the value succeeds; the declared length is not yet validated against available data.
assertEquals(START_SCALAR, cursor.nextValue());
// Filling the value cannot complete because the stream is exhausted long before the declared length is
// reached. The cursor must fail cleanly without over-allocating or hanging.
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();
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void expectUseAfterCloseToHaveNoEffect(boolean constructFromBytes) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package com.amazon.ion.system;

import com.amazon.ion.util.GzipStreamInterceptor;
import com.amazon.ion.util.InputStreamInterceptor;
import org.junit.jupiter.api.Test;

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

import static org.junit.jupiter.api.Assertions.*;

/**
* Tests for the GZIP decompression toggle API on {@link IonReaderBuilder}.
*/
class IonReaderBuilderGzipToggleTest {

@Test
void defaultBuilderHasGzipEnabled() {
IonReaderBuilder builder = IonReaderBuilder.standard();
assertTrue(builder.isGzipDecompressionEnabled());
}

@Test
void getInputStreamInterceptorsIncludesGzipWhenEnabled() {
IonReaderBuilder builder = IonReaderBuilder.standard();
List<InputStreamInterceptor> interceptors = builder.getInputStreamInterceptors();
boolean hasGzip = interceptors.stream().anyMatch(i -> i instanceof GzipStreamInterceptor);
assertTrue(hasGzip, "Expected GzipStreamInterceptor in interceptor list when GZIP is enabled");
}

@Test
void getInputStreamInterceptorsExcludesGzipWhenDisabled() {
IonReaderBuilder builder = IonReaderBuilder.standard()
.withGzipDecompressionEnabled(false);
List<InputStreamInterceptor> interceptors = builder.getInputStreamInterceptors();
boolean hasGzip = interceptors.stream().anyMatch(i -> i instanceof GzipStreamInterceptor);
assertFalse(hasGzip, "Expected no GzipStreamInterceptor in interceptor list when GZIP is disabled");
}

@Test
void customInterceptorsPreservedWhenGzipDisabled() {
InputStreamInterceptor customInterceptor = new InputStreamInterceptor() {
@Override
public String formatName() {
return "custom";
}

@Override
public int numberOfBytesNeededToDetermineMatch() {
return 2;
}

@Override
public boolean isMatch(byte[] candidate, int offset, int length) {
return false;
}

@Override
public InputStream newInputStream(InputStream interceptedStream) throws IOException {
return interceptedStream;
}
};

IonReaderBuilder builder = IonReaderBuilder.standard()
.addInputStreamInterceptor(customInterceptor)
.withGzipDecompressionEnabled(false);

List<InputStreamInterceptor> interceptors = builder.getInputStreamInterceptors();

// Custom interceptor should still be present
assertTrue(interceptors.contains(customInterceptor),
"Custom interceptor should be preserved when GZIP is disabled");

// GZIP interceptor should be excluded
boolean hasGzip = interceptors.stream().anyMatch(i -> i instanceof GzipStreamInterceptor);
assertFalse(hasGzip, "GzipStreamInterceptor should be excluded when GZIP is disabled");
}

@Test
void customInterceptorsPreservedWhenGzipEnabled() {
InputStreamInterceptor customInterceptor = new InputStreamInterceptor() {
@Override
public String formatName() {
return "custom";
}

@Override
public int numberOfBytesNeededToDetermineMatch() {
return 2;
}

@Override
public boolean isMatch(byte[] candidate, int offset, int length) {
return false;
}

@Override
public InputStream newInputStream(InputStream interceptedStream) throws IOException {
return interceptedStream;
}
};

IonReaderBuilder builder = IonReaderBuilder.standard()
.addInputStreamInterceptor(customInterceptor)
.withGzipDecompressionEnabled(true);

List<InputStreamInterceptor> interceptors = builder.getInputStreamInterceptors();

// Custom interceptor should be present
assertTrue(interceptors.contains(customInterceptor),
"Custom interceptor should be preserved when GZIP is enabled");

// GZIP interceptor should also be present
boolean hasGzip = interceptors.stream().anyMatch(i -> i instanceof GzipStreamInterceptor);
assertTrue(hasGzip, "GzipStreamInterceptor should be present when GZIP is enabled");
}

@Test
void withGzipDecompressionEnabledOnImmutableBuilderReturnsMutableCopy() {
IonReaderBuilder immutableBuilder = IonReaderBuilder.standard().immutable();
IonReaderBuilder result = immutableBuilder.withGzipDecompressionEnabled(false);

// The result should be a different instance (mutable copy)
assertNotSame(immutableBuilder, result);
// The original should still have GZIP enabled
assertTrue(immutableBuilder.isGzipDecompressionEnabled());
// The new builder should have GZIP disabled
assertFalse(result.isGzipDecompressionEnabled());
}

@Test
void copyConstructorPreservesGzipFlag() {
IonReaderBuilder original = IonReaderBuilder.standard()
.withGzipDecompressionEnabled(false);
IonReaderBuilder copy = original.copy();

assertFalse(copy.isGzipDecompressionEnabled(),
"Copy should preserve the gzipDecompressionEnabled flag");
}

@Test
void setGzipDecompressionEnabledOnMutableBuilder() {
IonReaderBuilder builder = IonReaderBuilder.standard();
builder.setGzipDecompressionEnabled(false);
assertFalse(builder.isGzipDecompressionEnabled());
}

@Test
void setGzipDecompressionEnabledOnImmutableBuilderThrows() {
IonReaderBuilder immutableBuilder = IonReaderBuilder.standard().immutable();
assertThrows(UnsupportedOperationException.class, () -> {
immutableBuilder.setGzipDecompressionEnabled(false);
});
}
}
Loading