Skip to content

Commit

Permalink
Ensures the binary IonReader throws when a value method that returns …
Browse files Browse the repository at this point in the history
…a primitive type is invoked on a null value.
  • Loading branch information
tgregg committed Jan 17, 2024
1 parent 78537ce commit 78e03d3
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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);

Check warning on line 635 in src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java#L635

Added line #L635 was not covered by tests
}
if (valueTid.type == IonType.INT) {
if (valueTid.length == 0) {
return 0;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);

Check warning on line 700 in src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java#L700

Added line #L700 was not covered by tests
}
if (valueTid.type == IonType.FLOAT) {
prepareScalar();
int length = (int) (valueMarker.endIndex - valueMarker.startIndex);
if (length == 0) {
Expand All @@ -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.");

Check warning on line 727 in src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java#L727

Added line #L727 was not covered by tests
}
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -391,4 +392,57 @@ public void expectMissingTimestampBodyToFailCleanly() {
assertThrows(IonException.class, reader::timestampValue);
reader.close();
}

@Test
public void expectAttemptToParseNullScalarAsJavaPrimitiveToFailCleanly() {
IonReaderContinuableCoreBinary reader = initializeReader(
true,
0xE0, 0x01, 0x00, 0xEA,
0x0F, // null.null
0x1F, // null.bool
0x2F, // null.int
0x3F, // null.int
0x4F, // null.float
0x5F // null.decimal
);
// 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.
reader.nextValue();
assertEquals(IonType.NULL, reader.getType());
assertThrows(IllegalStateException.class, reader::intValue);
reader.nextValue();
assertEquals(IonType.BOOL, reader.getType());
assertThrows(IllegalStateException.class, reader::booleanValue);
reader.nextValue();
assertEquals(IonType.INT, reader.getType());
assertThrows(IllegalStateException.class, reader::intValue);
assertThrows(IllegalStateException.class, reader::longValue);
assertThrows(IllegalStateException.class, reader::doubleValue);
assertNull(reader.bigIntegerValue());
assertNull(reader.decimalValue());
assertNull(reader.bigDecimalValue());
reader.nextValue();
assertEquals(IonType.INT, reader.getType());
assertThrows(IllegalStateException.class, reader::intValue);
assertThrows(IllegalStateException.class, reader::longValue);
assertThrows(IllegalStateException.class, reader::doubleValue);
assertNull(reader.bigIntegerValue());
assertNull(reader.decimalValue());
assertNull(reader.bigDecimalValue());
reader.nextValue();
assertEquals(IonType.FLOAT, reader.getType());
assertThrows(IllegalStateException.class, reader::doubleValue);
assertThrows(IllegalStateException.class, reader::longValue);
assertThrows(IllegalStateException.class, reader::intValue);
assertNull(reader.bigIntegerValue());
reader.nextValue();
assertEquals(IonType.DECIMAL, reader.getType());
assertNull(reader.decimalValue());
assertNull(reader.bigDecimalValue());
assertThrows(IllegalStateException.class, reader::doubleValue);
assertThrows(IllegalStateException.class, reader::longValue);
assertThrows(IllegalStateException.class, reader::intValue);
reader.close();
}
}

0 comments on commit 78e03d3

Please sign in to comment.