From 7194eee457dfbf38dea54649457340d16540d9d6 Mon Sep 17 00:00:00 2001 From: Michael Ackley Date: Fri, 6 Sep 2024 17:13:45 -0500 Subject: [PATCH 1/6] Check logon auth before resetting store --- session.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/session.go b/session.go index 5320f49b9..476ae551a 100644 --- a/session.go +++ b/session.go @@ -507,6 +507,12 @@ func (s *session) handleLogon(msg *Message) error { } } + nextSenderMsgNumAtLogonReceived := s.store.NextSenderMsgSeqNum() + + if err := s.verifyIgnoreSeqNumTooHigh(msg); err != nil { + return err + } + var resetSeqNumFlag FIXBoolean if err := msg.Body.GetField(tagResetSeqNumFlag, &resetSeqNumFlag); err == nil { if resetSeqNumFlag { @@ -517,18 +523,12 @@ func (s *session) handleLogon(msg *Message) error { } } - nextSenderMsgNumAtLogonReceived := s.store.NextSenderMsgSeqNum() - if resetStore { if err := s.store.Reset(); err != nil { return err } } - if err := s.verifyIgnoreSeqNumTooHigh(msg); err != nil { - return err - } - if !s.InitiateLogon { if !s.HeartBtIntOverride { var heartBtInt FIXInt From 2e7e0777f9ab8a46dc61f84cc1ee987ba53f3e0a Mon Sep 17 00:00:00 2001 From: Michael Ackley Date: Mon, 9 Sep 2024 17:30:14 -0500 Subject: [PATCH 2/6] Modify msg verify checks --- session.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/session.go b/session.go index 476ae551a..933b982b3 100644 --- a/session.go +++ b/session.go @@ -509,7 +509,8 @@ func (s *session) handleLogon(msg *Message) error { nextSenderMsgNumAtLogonReceived := s.store.NextSenderMsgSeqNum() - if err := s.verifyIgnoreSeqNumTooHigh(msg); err != nil { + // Make sure this is a valid session before resetting the store. + if err := s.verifyMsgAgainstAppImpl(msg); err != nil { return err } @@ -529,6 +530,12 @@ func (s *session) handleLogon(msg *Message) error { } } + // Verify seq num too high but dont check against app implementation since we just did that. + // Don't need to double check. + if err := s.verifySelect(msg, false, true, false); err != nil { + return err + } + if !s.InitiateLogon { if !s.HeartBtIntOverride { var heartBtInt FIXInt @@ -586,18 +593,18 @@ func (s *session) initiateLogoutInReplyTo(reason string, inReplyTo *Message) (er } func (s *session) verify(msg *Message) MessageRejectError { - return s.verifySelect(msg, true, true) + return s.verifySelect(msg, true, true, true) } func (s *session) verifyIgnoreSeqNumTooHigh(msg *Message) MessageRejectError { - return s.verifySelect(msg, false, true) + return s.verifySelect(msg, false, true, true) } func (s *session) verifyIgnoreSeqNumTooHighOrLow(msg *Message) MessageRejectError { - return s.verifySelect(msg, false, false) + return s.verifySelect(msg, false, false, true) } -func (s *session) verifySelect(msg *Message, checkTooHigh bool, checkTooLow bool) MessageRejectError { +func (s *session) verifySelect(msg *Message, checkTooHigh bool, checkTooLow bool, checkAppImpl bool) MessageRejectError { if reject := s.checkBeginString(msg); reject != nil { return reject } @@ -626,6 +633,14 @@ func (s *session) verifySelect(msg *Message, checkTooHigh bool, checkTooLow bool } } + if checkAppImpl { + return s.verifyMsgAgainstAppImpl(msg) + } + + return nil +} + +func (s *session) verifyMsgAgainstAppImpl(msg *Message) MessageRejectError { if s.Validator != nil { if reject := s.Validator.Validate(msg); reject != nil { return reject From dc7a7bfc7000c45117638252fa6e7614c3b3d339 Mon Sep 17 00:00:00 2001 From: Michael Ackley Date: Mon, 9 Sep 2024 20:22:03 -0500 Subject: [PATCH 3/6] Resolve failing unit test --- in_session.go | 4 ++-- logon_state_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/in_session.go b/in_session.go index 6edbb8409..5445967cd 100644 --- a/in_session.go +++ b/in_session.go @@ -87,7 +87,7 @@ func (state inSession) Timeout(session *session, event internal.Event) (nextStat } func (state inSession) handleLogout(session *session, msg *Message) (nextState sessionState) { - if err := session.verifySelect(msg, false, false); err != nil { + if err := session.verifySelect(msg, false, false, true); err != nil { return state.processReject(session, msg, err) } @@ -154,7 +154,7 @@ func (state inSession) handleSequenceReset(session *session, msg *Message) (next } } - if err := session.verifySelect(msg, bool(gapFillFlag), bool(gapFillFlag)); err != nil { + if err := session.verifySelect(msg, bool(gapFillFlag), bool(gapFillFlag), true); err != nil { return state.processReject(session, msg, err) } diff --git a/logon_state_test.go b/logon_state_test.go index ee47a2be6..4fe952743 100644 --- a/logon_state_test.go +++ b/logon_state_test.go @@ -358,6 +358,7 @@ func (s *LogonStateTestSuite) TestFixMsgInLogonSeqNumTooLow() { logon.Body.SetField(tagHeartBtInt, FIXInt(32)) logon.Header.SetInt(tagMsgSeqNum, 1) + s.MockApp.On("FromAdmin").Return(nil) s.MockApp.On("ToAdmin") s.NextTargetMsgSeqNum(2) s.fixMsgIn(s.session, logon) From fad0176e627de4c56e4eac3e6bc71d1638bf2a56 Mon Sep 17 00:00:00 2001 From: Michael Ackley Date: Mon, 9 Sep 2024 20:29:42 -0500 Subject: [PATCH 4/6] Linter --- session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session.go b/session.go index 933b982b3..d9bbfba65 100644 --- a/session.go +++ b/session.go @@ -530,7 +530,7 @@ func (s *session) handleLogon(msg *Message) error { } } - // Verify seq num too high but dont check against app implementation since we just did that. + // Verify seq num too high but dont check against app implementation since we just did that. // Don't need to double check. if err := s.verifySelect(msg, false, true, false); err != nil { return err From f1f8c0efa5992022ce60ae7c63bc589e9e0826e2 Mon Sep 17 00:00:00 2001 From: Michael Ackley Date: Mon, 9 Sep 2024 20:33:41 -0500 Subject: [PATCH 5/6] Linter fix --- session.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/session.go b/session.go index d9bbfba65..cdcb8603d 100644 --- a/session.go +++ b/session.go @@ -532,7 +532,7 @@ func (s *session) handleLogon(msg *Message) error { // Verify seq num too high but dont check against app implementation since we just did that. // Don't need to double check. - if err := s.verifySelect(msg, false, true, false); err != nil { + if err := s.verifyIgnoreSeqNumTooHigh(msg); err != nil { return err } @@ -597,7 +597,7 @@ func (s *session) verify(msg *Message) MessageRejectError { } func (s *session) verifyIgnoreSeqNumTooHigh(msg *Message) MessageRejectError { - return s.verifySelect(msg, false, true, true) + return s.verifySelect(msg, false, true, false) } func (s *session) verifyIgnoreSeqNumTooHighOrLow(msg *Message) MessageRejectError { From bb3e854d79346a9c77ea5592753e7833926625c0 Mon Sep 17 00:00:00 2001 From: Michael Ackley Date: Tue, 10 Sep 2024 15:58:21 -0500 Subject: [PATCH 6/6] Adds unit test --- logon_state_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/logon_state_test.go b/logon_state_test.go index 4fe952743..f5c871ea6 100644 --- a/logon_state_test.go +++ b/logon_state_test.go @@ -226,6 +226,27 @@ func (s *LogonStateTestSuite) TestFixMsgInLogonInitiateLogonExpectResetSeqNum() s.NextSenderMsgSeqNum(2) } +func (s *LogonStateTestSuite) TestFixMsgInLogonInitiateLogonRejectedSeqNumNotReset() { + s.session.InitiateLogon = true + s.session.sentReset = true + s.Require().Nil(s.store.IncrNextSenderMsgSeqNum()) + + logon := s.Logon() + logon.Body.SetField(tagHeartBtInt, FIXInt(32)) + logon.Body.SetField(tagResetSeqNumFlag, FIXBoolean(true)) + + s.MockApp.On("FromAdmin").Return(RejectLogon{"reject message"}) + s.MockApp.On("OnLogout") + s.MockApp.On("ToAdmin") + s.fixMsgIn(s.session, logon) + + s.MockApp.AssertExpectations(s.T()) + s.State(latentState{}) + + s.NextTargetMsgSeqNum(2) + s.NextSenderMsgSeqNum(3) +} + func (s *LogonStateTestSuite) TestFixMsgInLogonInitiateLogonUnExpectedResetSeqNum() { s.session.InitiateLogon = true s.session.sentReset = false