Session.generateReject methods may incorrectly increment nextTargetMsgSeqNum in some scenarios

Scenario is as follows:

• expecting 223
• received 227 Logon, which we accepted --> "MsgSeqNum too high, expecting 223 but received 227"
• sent resend request from 223 to 0
• received 228, which failed data dictionary validation. --> "Message 228 Rejected: Invalid MsgType"
• sent a Reject
• received 223 SequenceReset, w/ PossDupFlag=Y, GapFillFlag=Y and NextSeqNo=225. This gap fill was not applied.
• received 225 ExecutionReport w/ PossDupFlag=Y. --> "Rejected MsgSeqNum too high, expecting 224 but received 225", and "Already sent ResendRequest FROM: 223 TO: 226. Not sending another."

We were able to write a test to reproduce this. Tracing it through the debugger led to the following snippet in generateReject(Message, int, int):

            if (!msgType.equals(MsgType.LOGON) && !msgType.equals(MsgType.SEQUENCE_RESET)
                    && (msgSeqNum == getExpectedTargetNum() || !isPossibleDuplicate(message))) {
                state.incrNextTargetMsgSeqNum();

(There is a similar snippet in the other generateReject method.) The "bad" message 228 is not a Logon, not a SequenceReset and not a possible duplicate. Therefore the nextTargetMsgSeqNum is incremented, even though it should not be.

Consequently the Session thinks message 223 has too low a sequence number. Normally this would cause a log message and disconnect, but since it is flagged as a possible duplicate and is a SequenceReset, the message is silently dropped instead. See doTargetTooLow(Message) and validatePossDup(Message).

At this point we do not have a proposed fix. However, it seems clear that message 228 should not have caused an increment to nextTargetMsgSeqNum. The section "|| !isPossibleDuplicate(message)" in the snippet quoted above is a tad mysterious. Or it could be something deeper about how dictionary validation and sequence number validation should be ordered.