We incorrectly check for ApplVerID on admin messages

[philipwhiuk]

Describe the bug

We check for ApplVerID even on session level (admin) messages. We only exclude logon from this.
Per the FIXT.1.1 spec session level messages aren't versioned using the ApplVerID. So we shouldn't check them.

We don't even need the ApplVerID for the session level messages as we then use the session dictionary for validating the message.

To Reproduce

Apply MessageUtils.parse on a logout message.

Expected behavior

We should process the message regardless.

Actual behavior*

We throw an InvalidMessage Exception in MessageUtils.getApplVerID

https://github.com/quickfix-j/quickfixj/blob/master/quickfixj-core/src/main/java/quickfix/MessageUtils.java#L136

https://github.com/quickfix-j/quickfixj/blob/master/quickfixj-core/src/main/java/quickfix/MessageUtils.java#L185

system information:

• OS: Linux
• Java version: Java 8
• QFJ Version: 2.2.0-SNAPSHOT

Additional context

This is commonly seen in the real world in the following scenario:

1. Logon
2. Logout with no ApplVerID due to bad sequence numbers with no default set for the session
3. InvalidMessage thrown

[chrjohn]

Good catch. I never really thought about this but the field is named ApplVerID so it is no surprise that it is not needed on session-level messages. :)

Just for the sake of completeness here is the excerpt from the spec you mentioned:

The use of the Explicit Application Version fields [ApplVerID(1128), ApplExtID(1156), CstmApplVerID(1129)] is not permitted on FIX Session Level Messages.

[chrjohn]

Sorry, but why do we check for ApplVerID on Logon when it is not permitted on session-level messages?

Edit: We check for the DefaultApplVerID on Logon, right?

[philipwhiuk]

Yeah we look for the DefaultApplVerID on Logons.

This scenario I think can really only happen in a case where your session is in a fairly broken state.

In our case we:

1. Connected to a malfunctioning FIX engine that didn't notice the TCP connection dropped and then resumed
2. So it tried to send us app messages
3. Rejected them for lack of ApplVerID (even we processed that we'd have rejected them for lack of logon).
4. It then sent a Logout which we also rejected but shouldn't have.

So it's not really a standard scenario. BUT we should still obey the spec :)