diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java index 9fe4e334b8..32b169ffa5 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java @@ -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; @@ -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. */ diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index 3d6140bc94..07e33af548 100644 --- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -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 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); + } }