Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repeating group decoder not cleaning reference to 'next' when decoding a new message #525

Closed
lucianoviana opened this issue Oct 23, 2024 · 5 comments

Comments

@lucianoviana
Copy link
Contributor

lucianoviana commented Oct 23, 2024

When a repeating group decode is called, it's not nullifying the reference of 'next' element. As a consequence of this, if a previous message was successfully processed and it had a number X of elements in the same repeating group, then any succeeding message that is decoded which has an invalid value in the repeating group count tag will not detect the malformed repeating group because of the existing 'next' refence from previous decoding.

Example: Consider the messages below with repeating groups given by tags 120 and 121:

String good = "8=FIX.4.4|9=71|35=0|115=abc|116=2|117=1.1|127=19700101-00:00:00.001|120=2|121=1|121=2|10=053|"
String bad_ = "8=FIX.4.4|9=71|35=0|115=abc|116=2|117=1.1|127=19700101-00:00:00.001|120=3|121=1|121=2|10=053|"

Decoder decoder = new Decoder();
decoder.decode(good);
decoder.reset();
decoder.decode(bad_);

then

decoder.validate() == true //INCORRECT

@lucianoviana
Copy link
Contributor Author

this has been fixed on #528

grafstrom pushed a commit to VermicFinTech/artio that referenced this issue Nov 20, 2024
@writeoncereadmany
Copy link

This fix makes the decoder no longer garbage free in the steady state: every time we receive a new message, we allocate a new repeating group decoder.

Can we fix this by, instead of nulling out the group decoder, just resetting its properties instead?

@lucianoviana
Copy link
Contributor Author

apologies for the undesirable effect - and thx for reaching out. I'll fix this in the way you suggested, @writeoncereadmany . Cheers

@lucianoviana
Copy link
Contributor Author

the bug is fixed - hope it's all good now 👍

@writeoncereadmany
Copy link

tyvm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants