Fix for 432: Wrap additional AssertionError from IonReader (#433)

FasterXML · Dec 30, 2023 · 80ae368 · 80ae368
1 parent 8437178
commit 80ae368
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 18 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
@@ -273,20 +273,16 @@ public String getText() throws IOException
             case VALUE_STRING:
                 try {
                     return _reader.stringValue();
-                } catch (UnknownSymbolException e) {
+                } catch (UnknownSymbolException
                     // stringValue() will throw an UnknownSymbolException if we're
                     // 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 | NullPointerException e) {
+                    | AssertionError | NullPointerException e
                     // AssertionError if we're trying to get the text with a symbol
                     // id less than or equals to 0.
                     // NullPointerException may also be thrown on invalid data
-                    String msg = e.getMessage();
-                    if (msg == null) {
-                        msg = "UNKNOWN ROOT CAUSE";
-                    }
-                    throw _constructError("Internal `IonReader` error: "+msg, e);
+                    ) {
+                    return _reportCorruptContent(e);
                 }
             case VALUE_NUMBER_INT:
             case VALUE_NUMBER_FLOAT:
@@ -576,8 +572,11 @@ public JsonToken nextToken() throws IOException
             type = _reader.next();
         } catch (IonException e) {
             return _reportCorruptContent(e);
+
+        } catch (IndexOutOfBoundsException | AssertionError e) {
             // [dataformats-binary#420]: IonJava leaks IOOBEs so:
-        } catch (IndexOutOfBoundsException e) {
+            // [dataformats-binary#432]: AssertionError if we're trying to get the text
+            //   with a symbol id less than or equals to 0.
             return _reportCorruptContent(e);
         }
         if (type == null) {
@@ -717,17 +716,25 @@ protected void _handleEOF() throws JsonParseException
         }
     }
 
-    private <T> T _reportCorruptContent(Exception e) throws IOException
+    private <T> T _reportCorruptContent(Throwable e) throws IOException
     {
-        final String msg = String.format("Corrupt content to decode; underlying failure: (%s) %s",
-                e.getClass().getName(), e.getMessage());
+        String origMsg = e.getMessage();
+        if (origMsg == null) {
+            origMsg = "[no exception message]";
+        }
+        final String msg = String.format("Corrupt content to decode; underlying `IonReader` problem: (%s) %s",
+                e.getClass().getName(), origMsg);
         throw _constructError(msg, e);
     }
 
-    private <T> T _reportCorruptNumber(Exception e) throws IOException
+    private <T> T _reportCorruptNumber(Throwable e) throws IOException
     {
-        final String msg = String.format("Corrupt Number value to decode; underlying failure: (%s) %s",
-                e.getClass().getName(), e.getMessage());
+        String origMsg = e.getMessage();
+        if (origMsg == null) {
+            origMsg = "[no exception message]";
+        }
+        final String msg = String.format("Corrupt Number value to decode; underlying `IonReader` problem: (%s) %s",
+                e.getClass().getName(), origMsg);
         throw _constructError(msg, e);
     }
 

diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz417_64721InvalidIonTest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz417_64721InvalidIonTest.java
@@ -27,7 +27,7 @@ public void testFuzz64721AssertionException() throws Exception {
            mapper.readValue("$0/", EnumFuzz.class);
            fail("Should not pass (invalid content)");
        } catch (StreamReadException e) {
-           assertThat(e.getMessage(), Matchers.containsString("Internal `IonReader` error"));
+           assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode"));
        }
     }
 }
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
@@ -23,7 +23,7 @@ public void testFuzz65065() throws Exception {
            MAPPER.readTree(new ByteArrayInputStream(bytes));
            fail("Should not pass (invalid content)");
        } catch (StreamReadException e) {
-           assertThat(e.getMessage(), Matchers.containsString("Internal `IonReader` error"));
+           assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode"));
        }
     }
 

diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz432_65273_AssertionTest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz432_65273_AssertionTest.java
@@ -0,0 +1,32 @@
+package com.fasterxml.jackson.dataformat.ion.fuzz;
+
+import java.io.InputStream;
+
+import org.hamcrest.Matchers;
+import org.junit.Test;
+
+import com.fasterxml.jackson.core.JsonParser;
+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#4432]
+public class Fuzz432_65273_AssertionTest
+{
+    private final IonFactory factory =
+        IonFactory.builderForTextualWriters().build();
+
+    @Test
+    public void testFuzz65273() throws Exception {
+       try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65273.ion")) {
+           try (JsonParser p = factory.createParser(in)) {
+               p.nextToken();
+           }
+           fail("Should not pass (invalid content)");
+       } catch (StreamReadException e) {
+           assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode; underlying"));
+       }
+    }
+}
diff --git a/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz_65062_VarintTest.java b/ion/src/test/java/com/fasterxml/jackson/dataformat/ion/fuzz/Fuzz_65062_VarintTest.java
@@ -33,7 +33,7 @@ public void testFuzz65062_Varint() throws Exception {
            fail("Should not pass (invalid content)");
        } catch (StreamReadException e) {
            // 21-Dec-2023, tatu: Not 100% sure why we won't get Number-specific fail but:
-           assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode; underlying failure"));
+           assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode; underlying"));
        }
     }
 }
diff --git a/ion/src/test/resources/data/fuzz-65273.ion b/ion/src/test/resources/data/fuzz-65273.ion
@@ -0,0 +1 @@
+$ion_symbol_table::{imports:$0/////     ����t

diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -292,3 +292,6 @@ Arthur Chan (@arthurscchan)
   (2.17.0)
  * Contributed #426: (smile) `SmileParser` throws unexpected IOOBE for corrupt content
   (2.17.0)
+ * Contributed #432 (ion) More methods from `IonReader` could throw an unexpected
+   `AssertionError`
+  (2.17.0)
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -26,6 +26,8 @@ Active maintainers:
  (fix contributed by Arthur C)
 #426: (smile) `SmileParser` throws unexpected IOOBE for corrupt content
  (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)
 
 2.16.1 (24-Dec-2023)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		$ion_symbol_table::{imports:$0///// ��t
Expand Down