diff --git a/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java b/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java index cf7a727a6..98df0749e 100644 --- a/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java +++ b/ion/src/main/java/com/fasterxml/jackson/dataformat/ion/IonParser.java @@ -384,27 +384,47 @@ private void _verifyIsNumberToken() throws IOException @Override public NumberType getNumberType() throws IOException { - IonType type = _reader.getType(); - if (type != null) { - // Hmmh. Looks like Ion gives little bit looser definition here; - // harder to pin down exact type. But let's try some checks still. - switch (type) { - case DECIMAL: - //Ion decimals can be arbitrary precision, need to read as big decimal - return NumberType.BIG_DECIMAL; - case INT: - IntegerSize size = _reader.getIntegerSize(); - switch (size) { + if (_currToken == JsonToken.VALUE_NUMBER_INT + || _currToken == JsonToken.VALUE_NUMBER_FLOAT + // 30-Dec-2023, tatu: This is odd, but some current tests seem to + // expect this case to work when creating `IonParser` from `IonReader`, + // which does not seem to work without work-around like this: + || ((_currToken == null) && !isClosed())) { + IonType type = _reader.getType(); + if (type != null) { + // Hmmh. Looks like Ion gives little bit looser definition here; + // harder to pin down exact type. But let's try some checks still. + switch (type) { + case DECIMAL: + //Ion decimals can be arbitrary precision, need to read as big decimal + return NumberType.BIG_DECIMAL; case INT: - return NumberType.INT; - case LONG: - return NumberType.LONG; + final IntegerSize size; + // [dataformats-binary#434]: another problem with corrupt data handling. + // Temporary measure until this bug fixing is merged and published + // https://github.com/amazon-ion/ion-java/issues/685 + try { + size = _reader.getIntegerSize(); + } catch (IonException e) { + return _reportCorruptNumber(e); + } catch (NullPointerException e) { + return _reportCorruptContent(e); + } + if (size == null) { + _reportError("Current token (%s) not integer", _currToken); + } + switch (size) { + case INT: + return NumberType.INT; + case LONG: + return NumberType.LONG; + default: + return NumberType.BIG_INTEGER; + } + case FLOAT: + return NumberType.DOUBLE; default: - return NumberType.BIG_INTEGER; } - case FLOAT: - return NumberType.DOUBLE; - default: } } return null; diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java index a69c20538..6c00afe01 100644 --- a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java +++ b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/IonParserTest.java @@ -93,6 +93,10 @@ public void testFloatType() throws IOException { reader.stepIn(); // Step next. reader.next(); + // 30-Dec-2023, tatu: This is problematic as created parser is expected + // to point to `JsonToken.VALUE_NUMBER_FLOAT`, but `createParser()` + // does not initialize state. For now, `IonParser.getNumberType()` has + // special handling allowing this case but that should not be needed final IonParser floatParser = new IonFactory().createParser(reader); Assert.assertEquals(JsonParser.NumberType.DOUBLE, floatParser.getNumberType()); } diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz434_65268_65274_NPETest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz434_65268_65274_NPETest.java new file mode 100644 index 000000000..5b1ff4a93 --- /dev/null +++ b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz434_65268_65274_NPETest.java @@ -0,0 +1,59 @@ +package com.fasterxml.jackson.dataformat.ion.fuzz; + +import java.io.*; + +import org.hamcrest.Matchers; +import org.junit.Test; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.core.exc.StreamReadException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.ion.*; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +// [dataformats-binary#434] +public class Fuzz434_65268_65274_NPETest +{ + private final ObjectMapper ION_MAPPER = new IonObjectMapper(); + + // Test that used to fail on "getNumberType()" for `JsonToken.VALUE_NULL` + @Test + public void testFuzz65268() throws Exception { + try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65268.ion")) { + try (JsonParser p = ION_MAPPER.createParser(in)) { + assertEquals(JsonToken.VALUE_STRING, p.nextToken()); + p.getText(); + assertNull(p.nextTextValue()); + assertEquals(JsonToken.VALUE_NULL, p.currentToken()); + assertNull(p.getNumberType()); + } + } + } + + @Test + public void testFuzz65274Malformed() throws Exception { + try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65274.ion")) { + byte[] invalid = new byte[in.available()]; + new DataInputStream(in).readFully(invalid); + ION_MAPPER.readTree(new ByteArrayInputStream(invalid)); + fail("Should not pass (invalid content)"); + } catch (StreamReadException e) { + assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode")); + } + } + + @Test + public void testFuzz65274Eof() throws Exception { + try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65274.ion")) { + ION_MAPPER.readTree(in); + fail("Should not pass (invalid content)"); + } catch (StreamReadException e) { + assertThat(e.getMessage(), Matchers.containsString("Corrupt Number value to decode")); + } + } +} diff --git a/ion/src/test/resources/data/fuzz-65268.ion b/ion/src/test/resources/data/fuzz-65268.ion new file mode 100644 index 000000000..dd1539cbf --- /dev/null +++ b/ion/src/test/resources/data/fuzz-65268.ion @@ -0,0 +1,3 @@ +n + +null.intÿÿÿÿ~ÿÿÿttttt(þtõ740ôßßôßôßôßnÄÄÄÄ66}tt diff --git a/ion/src/test/resources/data/fuzz-65274.ion b/ion/src/test/resources/data/fuzz-65274.ion new file mode 100644 index 000000000..5e6ca06fd Binary files /dev/null and b/ion/src/test/resources/data/fuzz-65274.ion differ diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index e7246ea0c..8710ceb67 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -295,3 +295,5 @@ Arthur Chan (@arthurscchan) * Contributed #432 (ion) More methods from `IonReader` could throw an unexpected `AssertionError` (2.17.0) + * Contributed #434 (ion) Unexpected `NullPointerException` thrown from `IonParser::getNumberType()` + (2.17.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index c9254de34..89fcdc237 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -25,10 +25,12 @@ Active maintainers: #424: (ion) `IonReader` throws `NullPointerException` for unchecked invalid data (fix contributed by Arthur C) #426: (smile) `SmileParser` throws unexpected IOOBE for corrupt content - (fix contributed by Arthur C) + (fix contributed by Arthur C)-( #432: (ion) More methods from `IonReader` could throw an unexpected `AssertionError` (fix contributed by Arthur C) --(ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5) +#434 (ion) Unexpected `NullPointerException` thrown from `IonParser::getNumberType()` + (fix contributed by Arthur C) +- (ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5) 2.16.1 (24-Dec-2023)