Skip to content

Fix for 432: Wrap additional AssertionError from IonReader #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}
}
1 change: 1 addition & 0 deletions ion/src/test/resources/data/fuzz-65273.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$ion_symbol_table::{imports:$0///// ����t
Expand Down
3 changes: 3 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 2 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down