Description
I believe that you code generator has a bug in the decodeLength()
method of messages. Inside this method a new temporal instance of the message class is created in order to iterate over elements and provide length of the message (actually part of it that is understood by the parser). It goes like this:
MessageType skipper(m_buffer, m_offset, m_bufferLength, sbeBlockLength(), m_actingVersion, m_codecState);
As you can see, I am currently using precedence checks and it's possible that the bug is limited to this version.
The purpose of creating this temporal instance is to have a "fresh" instance that you can use to skip over variable length fields.
The problem is with two parts of this code:
- Call to
sbeBlockLength()
instead of usingm_actingBlockLength
. - Passing
m_codecState
Problem 1 is about passing a constant block length characteristic to the version of schema that was used by code generator. Instead the value from the received header should be used. This violates the forward compatibility paradigm as the received message may have a longer block length. To reproduce problem 1:
- Create a schema version 1 with few fields and use the generator to generate the code.
- Update the schema to version 2 and add few more fields. Generate a code for it.
- Generate a message using schema 2 code and parse it with schema 1 code.
- Call the decodeLength() method. Expect completely corrupted result.
Problem 2 is about codec state. Whenever you completely reset the parser, you should also reset the codec state. Otherwise precedence checks will throw and exception in a completely correct scenario. To reproduce:
- Create a schema with few
<data>
fields - Wrap message for decode.
- Read the first
<data>
field. This will change the codec state. - Call the
decodeLength()
method. Precedence checks will make it throw because the temporal instance is in an invalid state.
Suggestion for fixing
Since creation of a "fresh" instance exists in few places (for example operator <<
also creates a new temporal instance) I suggest wrapping this piece of code inside a new method to reduce duplication. You already have the sbeRewind()
method but the new one needs to return a new instance of the message class, without affecting the original instance:
SBE_NODISCARD MessageType sbeReset() {
return MessageType(
m_buffer,
m_offset,
m_bufferLength,
m_actingBlockLength,
m_actingVersion
#if defined(SBE_PRECEDENCE_CHECKS)
,CodecState::V0_BLOCK
#endif
);
}