Skip to content

Commit

Permalink
Fix leaking field name text in IonCursorBinary (#859)
Browse files Browse the repository at this point in the history
  • Loading branch information
popematt authored and tgregg committed Sep 9, 2024
1 parent 98bcbb8 commit b302b93
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 1 deletion.
5 changes: 5 additions & 0 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -1853,11 +1853,16 @@ private boolean checkContainerEnd() {
* Resets state specific to the current value.
*/
private void reset() {
valueTid = null;
valueMarker.typeId = null;
valueMarker.startIndex = -1;
valueMarker.endIndex = -1;
fieldSid = -1;
fieldTextMarker.typeId = null;
fieldTextMarker.startIndex = -1;
fieldTextMarker.endIndex = -1;
hasAnnotations = false;
annotationSequenceMarker.typeId = null;
annotationSequenceMarker.startIndex = -1;
annotationSequenceMarker.endIndex = -1;
if (refillableState != null) {
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/com/amazon/ion/Ion11Test.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ class Ion11Test {
val ION = IonSystemBuilder.standard().build()

fun ionText(text: String): Array<Any> = arrayOf(text, text.encodeToByteArray())
fun ionBinary(text: String): Array<Any> = arrayOf("Binary: ${text.slice(0..10)}", hexStringToByteArray(text))
fun ionBinary(name: String, bytes: String): Array<Any> = arrayOf(name, hexStringToByteArray(bytes))

// Arguments here are an array containing a String for the test case name, and a ByteArray of the test data.
@JvmStatic
fun ionData() = listOf(
ionBinary("{a:{$4:b}}", "E0 01 01 EA FD 0F 01 FF 61 D3 09 A1 62"),
ionText("""a::a::c::a::0 a::a::0"""),
ionText("""a::a::c::a::0 a::0"""),
ionText("""foo::bar::baz::false foo::0"""),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,23 @@ static ExpectationProvider<IonReaderContinuableTopLevelBinary> container(IonType
};
}

/**
* Steps in, asserts something about the content of a container, and then steps out.
*
* This enables us to use `next()` to move to a container value and check the annotations
* and/or field name of the value before checking the content of the container value.
*/
@SafeVarargs
static ExpectationProvider<IonReaderContinuableTopLevelBinary> inContainer(ExpectationProvider<IonReaderContinuableTopLevelBinary>... expectations) {
return consumer -> {
STEP_IN.accept(consumer);
for (Consumer<Consumer<Expectation<IonReaderContinuableTopLevelBinary>>> expectation : expectations) {
expectation.accept(consumer);
}
STEP_OUT.accept(consumer);
};
}

static ExpectationProvider<IonReaderContinuableTopLevelBinary> nullValue(IonType expectedType) {
return consumer -> consumer.accept(new Expectation<>(
String.format("null(%s)", expectedType),
Expand Down Expand Up @@ -5066,6 +5083,27 @@ public void readStruct_1_1(String inputBytes) throws Exception {
assertSimpleStructCorrectlyParsed(false, inputBytes);
}

@Test
public void ensureFieldNameStateDoesNotLeakIntoNestedStructs() throws Exception {
// This test case covers a very specific edge case where the field name was leaking from
// an outer struct to the first field of a nested struct, when the outer field name was
// an inline field name symbol, and the first inner field name was a given by SID.
// For example, { a: { $4: b } } was incorrectly being read as { a: { a: b } }
String data = "FD 0F 01 FF 61 D3 09 A1 62";
reader = readerForIon11(hexStringToByteArray(cleanCommentedHexBytes(data)), true);
assertSequence(
next(IonType.STRUCT), inContainer(
next(IonType.STRUCT), fieldName("a"), inContainer(
next(IonType.SYMBOL), fieldName("name"), symbolValue("b"),
next(null)
),
next(null)
),
next(null)
);
closeAndCount();
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void readMultipleNestedPrefixedStructs_1_1(boolean constructFromBytes) throws Exception {
Expand Down

0 comments on commit b302b93

Please sign in to comment.