From 3420d94867e98d43073f8b895d5b249d1b47d437 Mon Sep 17 00:00:00 2001
From: Matthew Pope <81593196+popematt@users.noreply.github.com>
Date: Wed, 15 May 2024 14:05:46 -0700
Subject: [PATCH] Fix leaking field name text in IonCursorBinary (#859)

---
 .../com/amazon/ion/impl/IonCursorBinary.java  |  5 +++
 src/test/java/com/amazon/ion/Ion11Test.kt     |  3 +-
 ...onReaderContinuableTopLevelBinaryTest.java | 38 +++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
index b831f1f86..3197fcefe 100644
--- a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
+++ b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java
@@ -1841,11 +1841,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) {
diff --git a/src/test/java/com/amazon/ion/Ion11Test.kt b/src/test/java/com/amazon/ion/Ion11Test.kt
index 9b88b4725..c9b08438c 100644
--- a/src/test/java/com/amazon/ion/Ion11Test.kt
+++ b/src/test/java/com/amazon/ion/Ion11Test.kt
@@ -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"""),
diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java
index 6078c14c4..a38252b01 100644
--- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java
+++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java
@@ -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),
@@ -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 {