From cab1ccf62b83f461810df09da5bac1a459d23a62 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Tue, 9 Jan 2024 17:01:42 -0800 Subject: [PATCH] Ensures the binary IonReader throws when a value method that returns a primitive type is invoked on a null value. --- .../impl/IonReaderContinuableCoreBinary.java | 20 ++++- .../IonReaderContinuableCoreBinaryTest.java | 89 +++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index 39813ffd10..df501feb7f 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -587,6 +587,9 @@ public BigDecimal bigDecimalValue() { value = minorVersion == 0 ? readBigDecimal_1_0() : readBigDecimal_1_1(); } } else if (valueTid.type == IonType.INT) { + if (valueTid.isNull) { + return null; + } prepareToConvertIntValue(); scalarConverter.cast(scalarConverter.get_conversion_fnid(_Private_ScalarConversions.AS_TYPE.decimal_value)); value = scalarConverter.getBigDecimal(); @@ -612,6 +615,9 @@ public Decimal decimalValue() { value = minorVersion == 0 ? readDecimal_1_0() : readDecimal_1_1(); } } else if (valueTid.type == IonType.INT) { + if (valueTid.isNull) { + return null; + } prepareToConvertIntValue(); scalarConverter.cast(scalarConverter.get_conversion_fnid(_Private_ScalarConversions.AS_TYPE.decimal_value)); value = scalarConverter.getDecimal(); @@ -625,7 +631,10 @@ public Decimal decimalValue() { @Override public long longValue() { long value; - if (valueTid.type == IonType.INT && !valueTid.isNull) { + if (valueTid.isNull) { + throwDueToInvalidType(IonType.INT); + } + if (valueTid.type == IonType.INT) { if (valueTid.length == 0) { return 0; } @@ -644,7 +653,7 @@ public long longValue() { value = scalarConverter.getLong(); scalarConverter.clear(); } else { - throw new IllegalStateException("longValue() may only be called on values of type int, float, or decimal."); + throw new IllegalStateException("longValue() may only be called on non-null values of type int, float, or decimal."); } return value; } @@ -687,7 +696,10 @@ public int intValue() { @Override public double doubleValue() { double value; - if (valueTid.type == IonType.FLOAT && !valueTid.isNull) { + if (valueTid.isNull) { + throwDueToInvalidType(IonType.FLOAT); + } + if (valueTid.type == IonType.FLOAT) { prepareScalar(); int length = (int) (valueMarker.endIndex - valueMarker.startIndex); if (length == 0) { @@ -712,7 +724,7 @@ public double doubleValue() { value = scalarConverter.getDouble(); scalarConverter.clear(); } else { - throw new IllegalStateException("doubleValue() may only be called on values of type float or decimal."); + throw new IllegalStateException("doubleValue() may only be called on non-null values of type float or decimal."); } return value; } diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java index 3de1ab1da0..874747ef31 100644 --- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableCoreBinaryTest.java @@ -27,6 +27,7 @@ import static com.amazon.ion.impl.IonCursorTestUtilities.startContainer; import static com.amazon.ion.impl.IonCursorTestUtilities.fillStringValue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; public class IonReaderContinuableCoreBinaryTest { @@ -391,4 +392,92 @@ public void expectMissingTimestampBodyToFailCleanly() { assertThrows(IonException.class, reader::timestampValue); reader.close(); } + + private static void assertReaderThrowsForAllPrimitives(IonReaderContinuableCoreBinary reader) { + // Note: ideally these would throw IonException instead of IllegalStateException, but there is long-standing + // precedent in IonJava for throwing IllegalStateException when these methods are used improperly. We maintain + // that convention for consistency. + assertThrows(IllegalStateException.class, reader::booleanValue); + assertThrows(IllegalStateException.class, reader::intValue); + assertThrows(IllegalStateException.class, reader::longValue); + assertThrows(IllegalStateException.class, reader::doubleValue); + } + + @Test + public void expectAttemptToParseNullNullAsJavaPrimitiveToFailCleanly() { + IonReaderContinuableCoreBinary reader = initializeReader( + true, + 0xE0, 0x01, 0x00, 0xEA, + 0x0F // null.null + ); + reader.nextValue(); + assertEquals(IonType.NULL, reader.getType()); + assertReaderThrowsForAllPrimitives(reader); + reader.close(); + } + + @Test + public void expectAttemptToParseNullBoolAsJavaPrimitiveToFailCleanly() { + IonReaderContinuableCoreBinary reader = initializeReader( + true, + 0xE0, 0x01, 0x00, 0xEA, + 0x1F // null.bool + ); + reader.nextValue(); + assertEquals(IonType.BOOL, reader.getType()); + assertReaderThrowsForAllPrimitives(reader); + reader.close(); + } + + @Test + public void expectAttemptToParseNullIntAsJavaPrimitiveToFailCleanly() { + IonReaderContinuableCoreBinary reader = initializeReader( + true, + 0xE0, 0x01, 0x00, 0xEA, + 0x2F, // null.int + 0x3F // null.int + ); + reader.nextValue(); + assertEquals(IonType.INT, reader.getType()); + assertReaderThrowsForAllPrimitives(reader); + assertNull(reader.bigIntegerValue()); + assertNull(reader.decimalValue()); + assertNull(reader.bigDecimalValue()); + reader.nextValue(); + assertEquals(IonType.INT, reader.getType()); + assertReaderThrowsForAllPrimitives(reader); + assertNull(reader.bigIntegerValue()); + assertNull(reader.decimalValue()); + assertNull(reader.bigDecimalValue()); + reader.close(); + } + + @Test + public void expectAttemptToParseNullFloatAsJavaPrimitiveToFailCleanly() { + IonReaderContinuableCoreBinary reader = initializeReader( + true, + 0xE0, 0x01, 0x00, 0xEA, + 0x4F // null.float + ); + reader.nextValue(); + assertEquals(IonType.FLOAT, reader.getType()); + assertReaderThrowsForAllPrimitives(reader); + assertNull(reader.bigIntegerValue()); + reader.close(); + } + + @Test + public void expectAttemptToParseNullDecimalAsJavaPrimitiveToFailCleanly() { + IonReaderContinuableCoreBinary reader = initializeReader( + true, + 0xE0, 0x01, 0x00, 0xEA, + 0x5F // null.decimal + ); + reader.nextValue(); + assertEquals(IonType.DECIMAL, reader.getType()); + assertNull(reader.decimalValue()); + assertNull(reader.bigDecimalValue()); + assertReaderThrowsForAllPrimitives(reader); + reader.close(); + } }