Merge branch 'master' into dependabot/github_actions/github/codeql-ac…

…tion-3.22.11

config/proguard/rules.pro

@@ -1,6 +1,5 @@
 # See https://www.guardsquare.com/manual/configuration/usage
--keep class !com.amazon.ion.shaded_.** { *; }
--keepattributes *Annotation*,Signature,InnerClasses,EnclosingMethod
+-keep,includecode class !com.amazon.ion.shaded_.** { *; }
 -dontoptimize
 -dontobfuscate
 -dontwarn java.sql.Timestamp

src/main/java/com/amazon/ion/BufferConfiguration.java

@@ -161,6 +161,11 @@ public int getMaximumBufferSize() {
          */
         public abstract OversizedValueHandler getNoOpOversizedValueHandler();
 
+        /**
+         * @return an {@link OversizedValueHandler} that always throws a runtime exception.
+         */
+        public abstract OversizedValueHandler getThrowingOversizedValueHandler();
+
         /**
          * @return the no-op {@link DataHandler} for the type of BufferConfiguration that this Builder builds.
          */
@@ -210,7 +215,7 @@ protected BufferConfiguration(Builder<Configuration, ?> builder) {
         }
         if (builder.getOversizedValueHandler() == null) {
             requireUnlimitedBufferSize();
-            oversizedValueHandler = builder.getNoOpOversizedValueHandler();
+            oversizedValueHandler = builder.getThrowingOversizedValueHandler();
         } else {
             oversizedValueHandler = builder.getOversizedValueHandler();
         }

src/main/java/com/amazon/ion/IonBufferConfiguration.java

@@ -8,6 +8,12 @@
  */
 public final class IonBufferConfiguration extends BufferConfiguration<IonBufferConfiguration> {
 
+    /**
+     * The standard configuration. This will be used unless the user chooses custom settings.
+     */
+    public static final IonBufferConfiguration DEFAULT =
+        IonBufferConfiguration.Builder.standard().build();
+
     /**
      * Functional interface for handling oversized symbol tables.
      */
@@ -31,6 +37,16 @@ public static final class Builder extends BufferConfiguration.Builder<IonBufferC
          */
         private static final int MINIMUM_MAX_VALUE_SIZE = 5;
 
+        private static void throwDueToUnexpectedOversizedValue() {
+            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."
+            );
+        }
+
+        private static final OversizedValueHandler THROWING_OVERSIZED_VALUE_HANDLER = Builder::throwDueToUnexpectedOversizedValue;
+        private static final OversizedSymbolTableHandler THROWING_OVERSIZED_SYMBOL_TABLE_HANDLER = Builder::throwDueToUnexpectedOversizedValue;
+
         /**
          * An OversizedValueHandler that does nothing.
          */
@@ -120,6 +136,11 @@ public OversizedValueHandler getNoOpOversizedValueHandler() {
             return NO_OP_OVERSIZED_VALUE_HANDLER;
         }
 
+        @Override
+        public OversizedValueHandler getThrowingOversizedValueHandler() {
+            return THROWING_OVERSIZED_VALUE_HANDLER;
+        }
+
         @Override
         public DataHandler getNoOpDataHandler() {
             return NO_OP_DATA_HANDLER;
@@ -132,6 +153,13 @@ public OversizedSymbolTableHandler getNoOpOversizedSymbolTableHandler() {
             return NO_OP_OVERSIZED_SYMBOL_TABLE_HANDLER;
         }
 
+        /**
+         * @return an OversizedSymbolTableHandler that always throws {@link IonException}.
+         */
+        public OversizedSymbolTableHandler getThrowingOversizedSymbolTableHandler() {
+            return THROWING_OVERSIZED_SYMBOL_TABLE_HANDLER;
+        }
+
         @Override
         public IonBufferConfiguration build() {
             return new IonBufferConfiguration(this);
@@ -151,7 +179,7 @@ private IonBufferConfiguration(Builder builder) {
         super(builder);
         if (builder.getOversizedSymbolTableHandler() == null) {
             requireUnlimitedBufferSize();
-            oversizedSymbolTableHandler = builder.getNoOpOversizedSymbolTableHandler();
+            oversizedSymbolTableHandler = builder.getThrowingOversizedSymbolTableHandler();
         } else {
             oversizedSymbolTableHandler = builder.getOversizedSymbolTableHandler();
         }

src/main/java/com/amazon/ion/impl/IonCursorBinary.java

@@ -278,6 +278,16 @@ private static class RefillableState {
     private long lastReportedByteTotal = 0;
 
 
+    /**
+     * @return the given configuration's DataHandler, or null if that DataHandler is a no-op.
+     */
+    private static BufferConfiguration.DataHandler getDataHandler(IonBufferConfiguration configuration) {
+        // Using null instead of a no-op handler enables a quick null check to skip calculating the amount of data
+        // processed, improving performance.
+        BufferConfiguration.DataHandler dataHandler = configuration.getDataHandler();
+        return dataHandler == IonBufferConfiguration.DEFAULT.getDataHandler() ? null : dataHandler;
+    }
+
     /**
      * Constructs a new fixed (non-refillable) cursor from the given byte array.
      * @param configuration the configuration to use. The buffer size and oversized value configuration are unused, as
@@ -292,7 +302,7 @@ private static class RefillableState {
         int offset,
         int length
     ) {
-        this.dataHandler = (configuration == null) ? null : configuration.getDataHandler();
+        this.dataHandler = getDataHandler(configuration);
         peekIndex = offset;
         valuePreHeaderIndex = offset;
         checkpoint = peekIndex;
@@ -310,12 +320,6 @@ private static class RefillableState {
         refillableState = null;
     }
 
-    /**
-     * The standard {@link IonBufferConfiguration}. This will be used unless the user chooses custom settings.
-     */
-    private static final IonBufferConfiguration STANDARD_BUFFER_CONFIGURATION =
-        IonBufferConfiguration.Builder.standard().build();
-
     /**
      * @param value a non-negative number.
      * @return the exponent of the next power of two greater than or equal to the given number.
@@ -352,13 +356,13 @@ static int nextPowerOfTwo(int value) {
     private static final IonBufferConfiguration[] FIXED_SIZE_CONFIGURATIONS;
 
     static {
-        int maxBufferSizeExponent = logBase2(STANDARD_BUFFER_CONFIGURATION.getInitialBufferSize());
+        int maxBufferSizeExponent = logBase2(IonBufferConfiguration.DEFAULT.getInitialBufferSize());
         FIXED_SIZE_CONFIGURATIONS = new IonBufferConfiguration[maxBufferSizeExponent + 1];
         for (int i = 0; i <= maxBufferSizeExponent; i++) {
             // Create a buffer configuration for buffers of size 2^i. The minimum size is 8: the smallest power of two
             // larger than the minimum buffer size allowed.
             int size = Math.max(8, (int) Math.pow(2, i));
-            FIXED_SIZE_CONFIGURATIONS[i] = IonBufferConfiguration.Builder.from(STANDARD_BUFFER_CONFIGURATION)
+            FIXED_SIZE_CONFIGURATIONS[i] = IonBufferConfiguration.Builder.from(IonBufferConfiguration.DEFAULT)
                 .withInitialBufferSize(size)
                 .withMaximumBufferSize(size)
                 .build();
@@ -370,6 +374,9 @@ static int nextPowerOfTwo(int value) {
      * @param configuration the configuration to validate.
      */
     private static void validate(IonBufferConfiguration configuration) {
+        if (configuration == null) {
+            throw new IllegalArgumentException("Buffer configuration must not be null.");
+        }
         if (configuration.getInitialBufferSize() < 1) {
             throw new IllegalArgumentException("Initial buffer size must be at least 1.");
         }
@@ -396,10 +403,10 @@ private static IonBufferConfiguration getFixedSizeConfigurationFor(
         if (alreadyReadLen > 0) {
             fixedBufferSize += alreadyReadLen;
         }
-        if (STANDARD_BUFFER_CONFIGURATION.getInitialBufferSize() > fixedBufferSize) {
+        if (IonBufferConfiguration.DEFAULT.getInitialBufferSize() > fixedBufferSize) {
             return FIXED_SIZE_CONFIGURATIONS[logBase2(fixedBufferSize)];
         }
-        return STANDARD_BUFFER_CONFIGURATION;
+        return IonBufferConfiguration.DEFAULT;
     }
 
     /**
@@ -416,19 +423,18 @@ private static IonBufferConfiguration getFixedSizeConfigurationFor(
         int alreadyReadOff,
         int alreadyReadLen
     ) {
-        this.dataHandler = (configuration == null) ? null : configuration.getDataHandler();
-        if (configuration == null) {
+        if (configuration == IonBufferConfiguration.DEFAULT) {
+            dataHandler = null;
             if (inputStream instanceof ByteArrayInputStream) {
                 // ByteArrayInputStreams are fixed-size streams. Clamp the reader's internal buffer size at the size of
                 // the stream to avoid wastefully allocating extra space that will never be needed. It is still
                 // preferable for the user to manually specify the buffer size if it's less than the default, as doing
                 // so allows this branch to be skipped.
                 configuration = getFixedSizeConfigurationFor((ByteArrayInputStream) inputStream, alreadyReadLen);
-            } else {
-                configuration = STANDARD_BUFFER_CONFIGURATION;
             }
         } else {
             validate(configuration);
+            dataHandler = getDataHandler(configuration);
         }
         peekIndex = 0;
         checkpoint = 0;

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;

src/main/java/com/amazon/ion/system/IonReaderBuilder.java

@@ -44,7 +44,7 @@ public abstract class IonReaderBuilder
 
     private IonCatalog catalog = null;
     private boolean isIncrementalReadingEnabled = false;
-    private IonBufferConfiguration bufferConfiguration = null;
+    private IonBufferConfiguration bufferConfiguration = IonBufferConfiguration.DEFAULT;
 
     protected IonReaderBuilder()
     {
@@ -249,6 +249,9 @@ public IonReaderBuilder withBufferConfiguration(IonBufferConfiguration configura
      */
     public void setBufferConfiguration(IonBufferConfiguration configuration) {
         mutationCheck();
+        if (configuration == null) {
+            throw new IllegalArgumentException("Configuration must not be null. To use the default configuration, provide IonBufferConfiguration.DEFAULT.");
+        }
         bufferConfiguration = configuration;
     }
 

src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java

@@ -38,6 +38,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.EOFException;
+import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.math.BigDecimal;
@@ -51,6 +52,7 @@
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.zip.GZIPInputStream;
 
 import static com.amazon.ion.BitUtils.bytes;
@@ -3604,4 +3606,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(IonBufferConfiguration.DEFAULT);
+        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(IonBufferConfiguration.DEFAULT);
+        }
+        // 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);
+    }
 }