[QFJ-750] Receiving Logout with too high or too low sequence causes messages to be lost Created: 11/Jul/13 Updated: 02/Apr/15 Resolved: 18/Dec/13 |
|
Status: | Closed |
Project: | QuickFIX/J |
Component/s: | Engine |
Affects Version/s: | 1.5.3 |
Fix Version/s: | 1.6.0 |
Type: | Bug | Priority: | Default |
Reporter: | Niall | Assignee: | Christoph John |
Resolution: | Fixed | Votes: | 0 |
Labels: | None |
Attachments: | QFJ-750-Session-with-test.patch QFJ-750-Session.java.patch |
Description |
For Loogout messages the "Too High" check is not done on the sequence number. If a Logout message with a sequence number that is too high is received, the NextTargetMsgSeqNum is still incremented anyway. This causes a message to be lost. For example say the NextTargetMsgSeqNum=500 and a Logout message with MsgSeqNum=550 is received NextTargetMsgSeqNum gets incremented to 501. Then a Logon message is received with MsgSeqNum=551 a ResendRequest is sent for 501 to 550 and MsgSeqNum 500 is lost/never received. In the nextLogout(Message) method I think modifying the increment statement to guard against this should avoid this situation: int msgSeqNum = message.getInt(MsgSeqNum.FIELD); if (!isTargetTooHigh(msgSeqNum)) { state.incrNextTargetMsgSeqNum(); } |
Comments |
Comment by Niall [ 12/Jul/13 ] |
Attaching a patch |
Comment by Christoph John [ 12/Jul/13 ] |
Good catch. Are you also able to provide a unit test? |
Comment by Niall [ 12/Jul/13 ] |
I will try and add a test in the next day or two |
Comment by Niall [ 13/Jul/13 ] |
Attaching patch with unit test |
Comment by Christoph John [ 18/Dec/13 ] |
This also applies for Logouts with too low a sequence number. And also for Rejects. |
Comment by Christoph John [ 18/Dec/13 ] |
Committed as http://sourceforge.net/p/quickfixj/code/1128 Thanks to Niall for the patch. Slightly enhanced it and added same check for incoming Rejects (which are also processed regardless of seqnum, just as Logout messages). |
Comment by Niall [ 23/Dec/13 ] |
Great, thanks for applying this |