[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: |
|
| 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. |
| 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 |