From b8b5c1943c1d28098fba5431352c73e0bf499dea Mon Sep 17 00:00:00 2001 From: Arthur Chan Date: Mon, 18 Dec 2023 08:24:47 +0000 Subject: [PATCH 1/4] Add null checking and wrap NPE Signed-off-by: Arthur Chan --- .../jackson/dataformat/ion/IonParser.java | 31 ++++++++++++ .../ion/fuzz/Fuzz424_65065_65126NPETest.java | 48 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java 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 479fba48a..465fd14bc 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 @@ -286,6 +286,13 @@ public String getText() throws IOException msg = "UNKNOWN ROOT CAUSE"; } throw _constructError("Internal `IonReader` error: "+msg, e); + } catch (NullPointerException e) { + // NullPointerException could be thrown on invalid data + String msg = e.getMessage(); + if (msg == null) { + msg = "UNKNOWN ROOT CAUSE"; + } + throw _constructError("Internal `IonReader` error: "+msg, e); } case VALUE_NUMBER_INT: case VALUE_NUMBER_FLOAT: @@ -331,31 +338,55 @@ public int getTextOffset() throws IOException { @Override public BigInteger getBigIntegerValue() throws IOException { + NumberType nt = getNumberType(); + if (nt == null) { + throw _constructError("Wrong number type: BigInteger not found.", null); + } return _reader.bigIntegerValue(); } @Override public BigDecimal getDecimalValue() throws IOException { + NumberType nt = getNumberType(); + if (nt == null) { + throw _constructError("Wrong number type: BigDecimal not found.", null); + } return _reader.bigDecimalValue(); } @Override public double getDoubleValue() throws IOException { + NumberType nt = getNumberType(); + if (nt == null) { + throw _constructError("Wrong number type: Double not found.", null); + } return _reader.doubleValue(); } @Override public float getFloatValue() throws IOException { + NumberType nt = getNumberType(); + if (nt == null) { + throw _constructError("Wrong number type: Float not found.", null); + } return (float) _reader.doubleValue(); } @Override public int getIntValue() throws IOException { + NumberType nt = getNumberType(); + if (nt == null) { + throw _constructError("Wrong number type: Int not found.", null); + } return _reader.intValue(); } @Override public long getLongValue() throws IOException { + NumberType nt = getNumberType(); + if (nt == null) { + throw _constructError("Wrong number type: Long not found.", null); + } return _reader.longValue(); } diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java new file mode 100644 index 000000000..9d327ff6b --- /dev/null +++ b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java @@ -0,0 +1,48 @@ +package com.fasterxml.jackson.dataformat.ion.fuzz; + +import java.io.ByteArrayInputStream; + +import org.hamcrest.Matchers; +import org.junit.Test; + +import com.fasterxml.jackson.core.exc.StreamReadException; +import com.fasterxml.jackson.dataformat.ion.*; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.fail; + +// [dataformats-binary#424] +public class Fuzz424_65065_65126NPETest +{ + @Test + public void testFuzz65065() throws Exception { + IonFactory f = IonFactory + .builderForBinaryWriters() + .build(); + + IonObjectMapper mapper = IonObjectMapper.builder(f).build(); + + try { + byte[] bytes = {(byte) -32, (byte) 1, (byte) 0, (byte) -22, (byte) 123, (byte) -112}; + mapper.readTree(f.createParser(new ByteArrayInputStream(bytes))); + fail("Should not pass (invalid content)"); + } catch (StreamReadException e) { + assertThat(e.getMessage(), Matchers.containsString("UNKNOWN ROOT CAUSE")); + } + } + + @Test + public void testFuzz65126() throws Exception { + IonFactory f = IonFactory + .builderForBinaryWriters() + .build(); + + try { + byte[] bytes = {(byte) 1, (byte) 0}; + f.createParser(bytes).getDecimalValue(); + fail("Should not pass (invalid content)"); + } catch (StreamReadException e) { + assertThat(e.getMessage(), Matchers.containsString("Wrong number type")); + } + } +} From badfce428e39100fc29f11312a36450e86929d20 Mon Sep 17 00:00:00 2001 From: Arthur Chan Date: Mon, 18 Dec 2023 09:55:47 +0000 Subject: [PATCH 2/4] Fix unit test Signed-off-by: Arthur Chan --- .../jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java index 9d327ff6b..c3696cdc4 100644 --- a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java +++ b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java @@ -27,7 +27,7 @@ public void testFuzz65065() throws Exception { mapper.readTree(f.createParser(new ByteArrayInputStream(bytes))); fail("Should not pass (invalid content)"); } catch (StreamReadException e) { - assertThat(e.getMessage(), Matchers.containsString("UNKNOWN ROOT CAUSE")); + assertThat(e.getMessage(), Matchers.containsString("Internal `IonReader` error")); } } From 36dac902117e3f1c6feb408aee8b9e3e9c1d7976 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 18 Dec 2023 19:13:47 -0800 Subject: [PATCH 3/4] Minor simplification of catch block --- .../fasterxml/jackson/dataformat/ion/IonParser.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) 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 465fd14bc..1d9b8cc3c 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 @@ -278,16 +278,10 @@ public String getText() throws IOException // trying to get the text for a symbol id that cannot be resolved. // stringValue() has an assert statement which could throw an throw _constructError(e.getMessage(), e); - } catch (AssertionError e) { + } catch (AssertionError | NullPointerException e) { // AssertionError if we're trying to get the text with a symbol // id less than or equals to 0. - String msg = e.getMessage(); - if (msg == null) { - msg = "UNKNOWN ROOT CAUSE"; - } - throw _constructError("Internal `IonReader` error: "+msg, e); - } catch (NullPointerException e) { - // NullPointerException could be thrown on invalid data + // NullPointerException may also be thrown on invalid data String msg = e.getMessage(); if (msg == null) { msg = "UNKNOWN ROOT CAUSE"; From 3a007a4668457153d1495e266eec43c442fad2c4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 18 Dec 2023 19:55:46 -0800 Subject: [PATCH 4/4] Add release notes, change number state checking slightly --- .../jackson/dataformat/ion/IonParser.java | 40 ++++++++----------- .../ion/fuzz/Fuzz424_65065_65126NPETest.java | 2 +- release-notes/CREDITS-2.x | 3 ++ release-notes/VERSION-2.x | 2 + 4 files changed, 22 insertions(+), 25 deletions(-) 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 1d9b8cc3c..e7ecd5e49 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 @@ -332,58 +332,50 @@ public int getTextOffset() throws IOException { @Override public BigInteger getBigIntegerValue() throws IOException { - NumberType nt = getNumberType(); - if (nt == null) { - throw _constructError("Wrong number type: BigInteger not found.", null); - } + _verifyIsNumberToken(); return _reader.bigIntegerValue(); } @Override public BigDecimal getDecimalValue() throws IOException { - NumberType nt = getNumberType(); - if (nt == null) { - throw _constructError("Wrong number type: BigDecimal not found.", null); - } + _verifyIsNumberToken(); return _reader.bigDecimalValue(); } @Override public double getDoubleValue() throws IOException { - NumberType nt = getNumberType(); - if (nt == null) { - throw _constructError("Wrong number type: Double not found.", null); - } + _verifyIsNumberToken(); return _reader.doubleValue(); } @Override public float getFloatValue() throws IOException { - NumberType nt = getNumberType(); - if (nt == null) { - throw _constructError("Wrong number type: Float not found.", null); - } + _verifyIsNumberToken(); return (float) _reader.doubleValue(); } @Override public int getIntValue() throws IOException { - NumberType nt = getNumberType(); - if (nt == null) { - throw _constructError("Wrong number type: Int not found.", null); - } + _verifyIsNumberToken(); return _reader.intValue(); } @Override public long getLongValue() throws IOException { - NumberType nt = getNumberType(); - if (nt == null) { - throw _constructError("Wrong number type: Long not found.", null); - } + _verifyIsNumberToken(); return _reader.longValue(); } + // @since 2.17 + private void _verifyIsNumberToken() throws IOException + { + if (_currToken != JsonToken.VALUE_NUMBER_INT && _currToken != JsonToken.VALUE_NUMBER_FLOAT) { + // Same as `ParserBase._parseNumericValue()` exception: + _reportError("Current token (%s) not numeric, can not use numeric value accessors", + _currToken); + } + } + @Override public NumberType getNumberType() throws IOException { diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java index c3696cdc4..5b054b14e 100644 --- a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java +++ b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz424_65065_65126NPETest.java @@ -42,7 +42,7 @@ public void testFuzz65126() throws Exception { f.createParser(bytes).getDecimalValue(); fail("Should not pass (invalid content)"); } catch (StreamReadException e) { - assertThat(e.getMessage(), Matchers.containsString("Wrong number type")); + assertThat(e.getMessage(), Matchers.containsString("Current token (null) not numeric")); } } } diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 471067b9c..3e451885f 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -287,3 +287,6 @@ Arthur Chan (@arthurscchan) (2.17.0) * Contributed #420: (ion) `IndexOutOfBoundsException` thrown by `IonReader` implementations (2.17.0) + * Contributed #424: (ion) `IonReader` throws `NullPointerException` for unchecked + invalid data + (2.17.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index bb5a266ca..fcdb42b28 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -22,6 +22,8 @@ Active maintainers: #420: (ion) `IndexOutOfBoundsException` thrown by `IonReader` implementations are not handled (contributed by Arthur C) +#424: (ion) `IonReader` throws `NullPointerException` for unchecked invalid data + (contributed by Arthur C) -(ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5) 2.16.0 (15-Nov-2023)