From 0255c1a50e3d8e58a6d92ad5e7d654f0ea7ee42d Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Wed, 13 Dec 2023 16:48:53 -0800 Subject: [PATCH] Corrects the struct FlexSym transition byte to FlexUInt 0 instead of byte 0. --- .../com/amazon/ion/impl/IonCursorBinary.java | 6 ++- ...onReaderContinuableTopLevelBinaryTest.java | 54 ++++++++++++++----- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java index 166279591c..8a0b00ff6a 100644 --- a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java @@ -10,6 +10,8 @@ import com.amazon.ion.IonType; import com.amazon.ion.IvmNotificationConsumer; import com.amazon.ion.SystemSymbols; +import com.amazon.ion.impl.bin.FlexInt; +import com.amazon.ion.impl.bin.Ion_1_1_Constants; import com.amazon.ion.impl.bin.OpCodes; import java.io.ByteArrayInputStream; @@ -1191,7 +1193,7 @@ private void uncheckedReadFieldName_1_1() { } else { // 0 in field name position of a SID struct indicates that all field names that follow are represented as // using FlexSyms. - if (buffer[(int) peekIndex] == 0) { + if (buffer[(int) peekIndex] == FlexInt.ZERO) { peekIndex++; parent.typeId = IonTypeID.STRUCT_WITH_FLEX_SYMS_ID; fieldSid = (int) uncheckedReadFlexSym_1_1(fieldTextMarker); @@ -1367,7 +1369,7 @@ private boolean slowReadFieldName_1_1() { } else { // 0 in field name position of a SID struct indicates that all field names that follow are represented as // using FlexSyms. - if (buffer[(int) peekIndex] == 0) { + if (buffer[(int) peekIndex] == FlexInt.ZERO) { peekIndex++; parent.typeId = IonTypeID.STRUCT_WITH_FLEX_SYMS_ID; fieldSid = (int) slowReadFlexSym_1_1(fieldTextMarker); diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index 67ae8ad6ed..9537c45812 100644 --- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -4495,7 +4495,7 @@ private void assertSimpleStructCorrectlyParsed(boolean constructFromBytes, Strin // FlexSym field names using SID type ID "FC | Variable Length SID struct \n" + "21 | Length = 16 \n" + - "00 | Switch to FlexSyms \n" + + "01 | Switch to FlexSyms \n" + "F9 6E 61 6D 65 | name \n" + "5E | true \n" + "F3 69 6D 70 6F 72 74 73 | imports \n " + @@ -4504,12 +4504,12 @@ private void assertSimpleStructCorrectlyParsed(boolean constructFromBytes, Strin "CC | Struct Length = 12 \n" + "09 | SID 4 \n" + "5E | true \n" + - "00 | Switch to FlexSyms \n" + + "01 | Switch to FlexSyms \n" + "F3 69 6D 70 6F 72 74 73 | imports \n " + "5F | false", // FlexSym then SID "C9 | Struct Length = 12 \n" + - "00 | Switch to FlexSyms \n" + + "01 | Switch to FlexSyms \n" + "F9 6E 61 6D 65 | name \n" + "5E | true \n" + "0D | SID 6 \n " + @@ -4574,14 +4574,15 @@ private void assertStructWithSymbolZeroFieldNamesCorrectlyParsed(boolean constru @ParameterizedTest @ValueSource(strings = { // SID 0 in fixed-length SID struct - "C8 | Struct Length = 8 \n" + - "01 | SID 0 \n" + + "CC | Struct Length = 12 \n" + + "01 | Switch to FlexSyms \n" + + "01 90 | FlexSym 0 \n" + "5E | true \n" + - "01 | FlexSym SID 0 \n" + + "01 90 | FlexSym SID 0 \n" + "5E | true \n" + "09 | FlexSym SID 4 (name) \n" + "5E | true \n" + - "01 | FlexSym SID 0 \n" + + "01 90 | FlexSym SID 0 \n" + "5E | true", // SID 0 in variable-length FlexSym struct "FD | Variable length SID struct \n" + @@ -4594,12 +4595,12 @@ private void assertStructWithSymbolZeroFieldNamesCorrectlyParsed(boolean constru "5E | true \n" + "01 90 | FlexSym SID 0 \n" + "5E | true", - // SID 0 before and after transitioning from SIDs to FlexSyms + // SID 0 in variable-length SID to FlexSyms "FC | Variable length SID struct \n" + - "17 | Length = FlexUInt 11 \n" + - "01 | SID 0 \n" + + "19 | Length = FlexUInt 12 \n" + + "01 | Switch to FlexSyms \n" + + "01 90 | SID 0 \n" + "5E | true \n" + - "00 | switch to FlexSym encoding \n" + "01 90 | FlexSym SID 0 \n" + "5E | true \n" + "09 | FlexSym SID 4 (name) \n" + @@ -4613,6 +4614,33 @@ public void readStructWithSymbolZeroFieldNames_1_1(String inputBytes) throws Exc assertStructWithSymbolZeroFieldNamesCorrectlyParsed(false, inputBytes); } + /** + * Verifies that the reader considers the given input to be incomplete. + */ + private void assertIncompleteInput(boolean constructFromBytes, String inputBytes) throws Exception { + reader = readerForIon11(hexStringToByteArray(cleanCommentedHexBytes(inputBytes)), constructFromBytes); + // When closed without receiving input that completes an incomplete value, the reader throws. + assertSequence(next(null)); + assertThrows(IonException.class, () -> reader.close()); + } + + @ParameterizedTest + @ValueSource(strings = { + // FlexUInt 0 in a fixed-length SID struct + "C3 | Fixed-length 3 SID struct \n" + + "01 | FlexUInt 0 \n" + + "50 | This should not be parsed as int 0, but rather the first byte in a FlexSym field name", + // FlexUInt 0 in a variable-length SID struct + "FC | Variable-length SID struct \n" + + "07 | FlexUInt length = 3 \n" + + "01 | FlexUInt 0 \n" + + "50 | This should not be parsed as int 0, but rather the first byte in a FlexSym field name" + }) + public void symbolIdStructWithFlexUIntZeroInFieldNameIsNotTreatedAsSidZero(String inputBytes) throws Exception { + assertIncompleteInput(true, inputBytes); + assertIncompleteInput(false, inputBytes); + } + /** * Checks that the reader correctly reads empty inline text, which requires a special FlexSym, as a field name. */ @@ -4631,13 +4659,13 @@ public void assertStructWithEmptyInlineFieldNamesCorrectlyParsed(boolean constru @ValueSource(strings = { // Empty field name in fixed-length SID struct "C4 | Struct Length = 4 \n" + - "00 | switch to FlexSym encoding \n" + + "01 | switch to FlexSym encoding \n" + "01 80 | FlexSym empty text \n" + "5F | false", // Empty field name in variable-length SID struct "FC | Variable length SID struct \n" + "09 | Length = 4 \n" + - "00 | switch to FlexSym encoding \n" + + "01 | switch to FlexSym encoding \n" + "01 80 | FlexSym empty text \n" + "5F | false", // Empty field name in variable-length FlexSym struct