Skip to content

Commit

Permalink
Handles the case where the reader unexpectedly encounters an oversize…
Browse files Browse the repository at this point in the history
…d value, throwing IonException with a helpful message.
  • Loading branch information
tgregg committed Dec 22, 2023
1 parent 56bada5 commit 5ae7ba8
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 2 deletions.
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.IonCatalog;
import com.amazon.ion.IonException;
import com.amazon.ion.IonReader;
Expand Down Expand Up @@ -154,15 +155,31 @@ class IonReaderContinuableApplicationBinary extends IonReaderContinuableCoreBina
}
}
if (mightBeSymbolTable) {
builder.getBufferConfiguration().getOversizedSymbolTableHandler().onOversizedSymbolTable();
requireBufferConfiguration(builder).getOversizedSymbolTableHandler().onOversizedSymbolTable();
terminate();
} else {
builder.getBufferConfiguration().getOversizedValueHandler().onOversizedValue();
requireBufferConfiguration(builder).getOversizedValueHandler().onOversizedValue();
}
}
);
}

/**
* Returns the reader's buffer configuration, throwing if null.
* @param builder the builder from which to retrieve the configuration.
* @return the non-null configuration.
*/
private static IonBufferConfiguration requireBufferConfiguration(IonReaderBuilder builder) {
IonBufferConfiguration bufferConfiguration = builder.getBufferConfiguration();
if (bufferConfiguration == null) {
throw new IonException("An oversized value was found even though no maximum size was configured. " +
"This is either due to data corruption or encountering a scalar larger than 2 GB. In the latter case, " +
"an IonBufferConfiguration can be configured to allow the reader to skip the oversized scalar."
);
}
return bufferConfiguration;
}

/**
* Reusable iterator over the annotations on the current value.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3604,4 +3604,144 @@ public void earlyStepOutNonIncremental(boolean constructFromBytes) throws Except
// However, the reader *must* fail if the user requests the next value, because the stream is incomplete.
assertThrows(IonException.class, () -> reader.next()); // Unexpected EOF
}

/**
* Verifies that the reader throws IonException when the provided code is executed over the given input.
* @param constructFromBytes whether to provide bytes (true) or an InputStream (false) to the reader.
* @param codeExpectedToThrow the code expected to throw IonException.
* @param input the input data.
*/
private void expectIonException(
boolean constructFromBytes,
Consumer<IonReader> codeExpectedToThrow,
int... input
) throws Exception {
readerBuilder = readerBuilder.withIncrementalReadingEnabled(false).withBufferConfiguration(null);
reader = readerFor(constructFromBytes, input);
assertThrows(IonException.class, () -> codeExpectedToThrow.accept(reader));
reader.close();
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void scalarValueLargerThan2GB(boolean constructFromBytes) throws Exception {
expectIonException(
constructFromBytes,
reader -> {
reader.next();
reader.longValue();
},
0x2E, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFF // Int with length larger than 2GB
);
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void scalarValueLargerThan2GBWithinSymbolTable(boolean constructFromBytes) throws Exception {
expectIonException(
constructFromBytes,
IonReader::next,
0xEB, 0x81, 0x83, // Annotation wrapper length 11 with one annotation: 3 ($ion_symbol_table)
0xD8, // Struct length 8
0x87, 0x2E, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFF // Field name 7 (symbols), int with length larger than 2GB
);
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void possibleSymbolTableStructDeclaresLengthLargerThan2GB(boolean constructFromBytes) throws Exception {
expectIonException(
constructFromBytes,
IonReader::next,
0xEB, 0x81, 0x83, // Annotation wrapper length 11 with one annotation: 3 ($ion_symbol_table)
0xDE, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFF, // Struct with length larger than 2GB
0x87, 0x20 // Field name 7 (symbols), int 0
);
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void possibleSymbolTableAnnotationDeclaresLengthLargerThan2GB(boolean constructFromBytes) throws Exception {
expectIonException(
constructFromBytes,
IonReader::next,
0xEE, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0xFF, // Annotation wrapper with length larger than 2GB
0x81, 0x83, // One annotation: 3 ($ion_symbol_table)
0xDE, 0x7F, 0x7F, 0x7F, 0x7F, 0x7F, 0x7C, // Struct with length larger than 2GB
0x87, 0x20 // Field name 7 (symbols), int 0
);
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void annotationSequenceLengthThatOverflowsBufferThrowsIonException(boolean constructFromBytes) throws Exception {
// This emulates a bad byte (0x52) replacing the annotation sequence length VarUInt in a symbol table annotation
// wrapper. This results in a declared annotation sequence length much larger than the total length of the
// stream. This should be detected and raised as an IonException.
expectIonException(
constructFromBytes,
IonReader::next,
0xEE, 0x9A, 0x52, 0x83, 0xDE, 0x96, 0x87
);
}

/**
* Verifies that corrupting each byte in the input data results in IonException, or nothing.
* @param constructFromBytes whether to provide bytes (true) or an InputStream (false) to the reader.
* @param incrementalReadingEnabled whether to enable incremental reading.
* @param emptyBufferConfiguration whether to set the buffer configuration to null (true), or use the standard test configuration (false).
*/
private void corruptEachByteThrowsIonException(
boolean constructFromBytes,
boolean incrementalReadingEnabled,
boolean emptyBufferConfiguration
) {
IonReaderBuilder builder = readerBuilder.copy();
builder = builder.withIncrementalReadingEnabled(incrementalReadingEnabled);
if (emptyBufferConfiguration) {
builder = builder.withBufferConfiguration(null);
}
// The following data begins with a symbol table, contains nested containers with length subfields, and
// contains a string, an integer, and blobs.
byte[] original = toBinary(
"{\n" +
" C:\"CRDR\",\n" +
" V:3,\n" +
" DR:{\n" +
" P:\"dummyPartitionKey\",\n" +
" D:{{ dGVzdERhdGE= }},\n" +
" S:{{ AeJA }},\n" +
" DC:{{ brzdAsJNBCzI3S63zno7Uw== }},\n" +
" HC:{{ f5C7STBM1f2S4SFkJWgbIizTYBE= }}\n" +
" }\n" +
"}"
);
byte[] corrupted = new byte[original.length];
for (int i = 0; i < corrupted.length; i++) {
// Fill with the original data
System.arraycopy(original, 0, corrupted, 0, original.length);
// Corrupt the byte at index i
corrupted[i] = 0x52;
// Ensure the corruption results in IonException or nothing.
reader = readerFor(builder, constructFromBytes, corrupted);
try {
TestUtils.deepRead(reader);
// Successful parsing is possible because the input contains blobs whose content is not inspected for corruption.
reader.close();
} catch (IonException e) {
// This is expected to be thrown when corruption is detected.
} catch (Exception f) {
fail("Expected IonException to be thrown, but was: ", f);
}
}
}

@ParameterizedTest(name = "constructFromBytes={0}")
@ValueSource(booleans = {true, false})
public void corruptEachByteThrowsIonException(boolean constructFromBytes) {
corruptEachByteThrowsIonException(constructFromBytes, true, true);
corruptEachByteThrowsIonException(constructFromBytes, true, false);
corruptEachByteThrowsIonException(constructFromBytes, false, true);
corruptEachByteThrowsIonException(constructFromBytes, false, false);
}
}

0 comments on commit 5ae7ba8

Please sign in to comment.