Skip to content

Commit

Permalink
HY-51: decoder reset missing to set next to null in groups decoder,…
Browse files Browse the repository at this point in the history
… leaving ghosts references when parsing new message with the same decoder instance - issue 525.
  • Loading branch information
lucianoviana committed Nov 7, 2024
1 parent 354423c commit 99f3d76
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -416,21 +416,28 @@ protected String resetGroup(final Entry entry)
" {\n" +
" for (final %2$s %6$s : %5$s.iterator())\n" +
" {\n" +
" %6$s.reset();\n" +
" if (%6$s.next() == null)\n" +
" {\n" +
" %6$s.reset();\n" +
" break;\n" +
" }\n" +
" else\n" +
" {\n" +
" %6$s.reset();\n" +
" }\n" +
" }\n" +
" %3$s = MISSING_INT;\n" +
" has%4$s = false;\n" +
" %7$s = null;\n" +
" }\n\n",
resetMethod,
decoderClassName(name),
formatPropertyName(numberField.name()),
numberField.name(),
iteratorFieldName(group),
formatPropertyName(decoderClassName(name)));
formatPropertyName(decoderClassName(name)),
formatPropertyName(name)
);
}
}

Expand Down Expand Up @@ -484,6 +491,9 @@ private String additionalReset(final boolean isGroup)
{
return
" buffer = null;\n" +
(isGroup ?
" next = null;\n" : ""
) +
" if (" + CODEC_VALIDATION_ENABLED + ")\n" +
" {\n" +
" invalidTagId = Decoder.NO_ERROR;\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public final class ExampleDictionary
"8=FIX.4.4\0019=77\00135=0\001115=abc\001116=2\001117=1.1\001127=19700101-00:00:00.001" +
"\001120=2\001121=1\001122=2\001123=1\001123=2\001121=2\001122=2\001123=3\001123=4\00110=063\001";

public static final String MULTI_ENTRY_NESTED_GROUP_MESSAGE_WITHOUT_NESTED_FIELDS =
public static final String MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS =
"8=FIX.4.4\0019=77\00135=0\001115=abc\001116=2\001117=1.1\001127=19700101-00:00:00.001" +
"\001120=2\001121=1\001121=2\00110=063\001";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.function.Consumer;
import java.util.function.Function;

import static java.lang.reflect.Modifier.isAbstract;
Expand Down Expand Up @@ -838,14 +839,21 @@ public void shouldReasonablyValidateGroupNumbersLessThanTheNumberOfElementsInThe
assertInvalid(decoder, INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP, 120);
}

//TODO this test was added to guide the bugfix
//this test was added for issue #525
@Test
public void shouldReasonablyValidateGroupNumbersLessThanTheNumberOfElementsInTheGroupList() throws Exception
{
final Decoder decoder = decodeHeartbeatWithRejectingUnknownFields(
REPEATING_GROUP_MESSAGE_WITH_THREE, REPEATING_GROUP_MESSAGE_WITH_TOO_HIGH_NUMBER_FIELD);
final Decoder decoder = (Decoder)heartbeatWithRejectingUnknownFields.getConstructor().newInstance();

// assertInvalid(decoder, INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP, 120);
decodeHeartbeatWithRejectingUnknownFields(
decoder, this::assertValid, REPEATING_GROUP_MESSAGE_WITH_THREE);
decodeHeartbeatWithRejectingUnknownFields(
decoder, decoder1 ->
assertInvalid(decoder1,
INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP,
120), REPEATING_GROUP_MESSAGE_WITH_TOO_HIGH_NUMBER_FIELD);
decodeHeartbeatWithRejectingUnknownFields(
decoder, this::assertValid, REPEATING_GROUP_MESSAGE_WITH_THREE);
}

@Test
Expand Down Expand Up @@ -1167,15 +1175,29 @@ public void shouldResetAllNestedRepeatingGroupEntries() throws Exception

decoder.reset();

decode(MULTI_ENTRY_NESTED_GROUP_MESSAGE_WITHOUT_NESTED_FIELDS, decoder);
decode(MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS, decoder);
assertEquals(2, getNoEgGroupGroupCounter(decoder));

group = getEgGroup(decoder);
assertNull(getNestedGroup(group));

group = next(group);
assertNull(getNestedGroup(group));
}

@Test
public void shouldHaveAllNestedRepeatingGroupEntriesNullifiedWhenMessageDoesNotHaveIt() throws Exception
{
Object group;

assertNestedRepeating(group, 1, CodecUtil.MISSING_INT, CodecUtil.MISSING_INT);
final Decoder decoder = decodeHeartbeat(MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS);
assertEquals(2, getNoEgGroupGroupCounter(decoder));

group = getEgGroup(decoder);
assertNull(getNestedGroup(group));

group = next(group);
assertNestedRepeating(group, 2, CodecUtil.MISSING_INT, CodecUtil.MISSING_INT);
assertNull(getNestedGroup(group));
}

@Test
Expand Down Expand Up @@ -1889,13 +1911,12 @@ Decoder decodeHeartbeatWithoutValidation(final String example) throws Exception
return decoder;
}

private Decoder decodeHeartbeatWithRejectingUnknownFields(final String... example) throws Exception
private Decoder decodeHeartbeatWithRejectingUnknownFields(final Decoder decoder,
final Consumer<Decoder> consumer,
final String example)
{
final Decoder decoder = (Decoder)heartbeatWithRejectingUnknownFields.getConstructor().newInstance();
for (final String s : example)
{
decode(s, decoder);
}
decode(example, decoder);
consumer.accept(decoder);
return decoder;
}

Expand All @@ -1909,6 +1930,7 @@ private Decoder decodeHeartbeatWithRejectingUnknownFields(final String example)
void decode(final String example, final Decoder decoder)
{
buffer.putAscii(1, example);
decoder.reset();
decoder.decode(buffer, 1, example.length());
}

Expand Down

0 comments on commit 99f3d76

Please sign in to comment.