Skip to content

Commit

Permalink
Corrects the struct FlexSym transition byte to FlexUInt 0 instead of …
Browse files Browse the repository at this point in the history 
…byte 0.
  • Loading branch information
@tgregg
tgregg committed Sep 9, 2024
1 parent 470c0e4 commit 9103bfc
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 15 deletions.
6 changes: 4 additions & 2 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,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;
Expand Down Expand Up @@ -1227,7 +1229,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);
Expand Down Expand Up @@ -1403,7 +1405,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);
Expand Down
54 changes: 41 additions & 13 deletions src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5003,7 +5003,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 " +
Expand All @@ -5012,12 +5012,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 " +
Expand Down Expand Up @@ -5082,14 +5082,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" +
Expand All @@ -5102,12 +5103,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" +
Expand All @@ -5121,6 +5122,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.
*/
Expand All @@ -5139,13 +5167,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
Expand Down

0 comments on commit 9103bfc

Please sign in to comment.