Skip to content

Conversation

@maxgawason
Copy link
Contributor

This PR fixes a bug introduced in #546 where a validation failure would cause every subsequent message to fail validation. #546 added validation for the length of boolean and char fields, asserting that the fix tag was only a single character long. Using Rule80A (tag 47) as an example, a buffer would be created for the decoder:

this.rule80AAsChars = new char[1];

When a message was decoded, the value of the fix tag would be copied into the buffer:

case 47:
   this.rule80A = buffer.getChar(valueOffset);
   this.rule80AAsChars = buffer.getChars(this.rule80AAsChars, valueOffset, valueLength);
   break;

getChars is designed to create a new, larger buffer if the field it attempts to read is too large for the current buffer:

public char[] getChars(final char[] oldBuffer, final int offset, final int length)
{
    final char[] resultBuffer = oldBuffer.length < length ? new char[length] : oldBuffer;
    for (int i = 0; i < length; i++)
    {
        resultBuffer[i] = getChar(i + offset);
    }
    return resultBuffer;
}

The validation would then check the size of the buffer length:

} else if (Validation.CODEC_VALIDATION_ENABLED && this.rule80AAsChars.length > 1) {

Because the buffer was never resized/reset for each fix message, anytime large tag value was sent (e.g. 47=AB) the buffer size would be increased and remain that size. Each subsequent check of ${tag}AsChars.length would then fail.

This PR removes the ${tag}AsChars field and replaces it with a ${tag}Length field for boolean and char fields that holds the length of the field to ensure subsequent messages with the correct length are valid. It maintains the same validation behavior while avoiding heap allocations for every message.

@lucianoviana
Copy link
Contributor

thx much @maxgawason - we're going to take a look

@lucianoviana lucianoviana merged commit 64319d9 into artiofix:master Jun 4, 2025
6 checks passed
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

Successfully merging this pull request may close these issues.

2 participants