Acceptor omits ResetSeqNumFlag(141) on Logon response when ResetOnLogon=Y (only echoes the inbound flag)

[foxbit-ruschel]

Version

github.com/quickfixgo/quickfix v0.9.10 (latest release as of writing). The code paths below are unchanged on main.

Summary

When an acceptor is configured with ResetOnLogon=Y and the initiator does not send ResetSeqNumFlag(141)=Y in its Logon, the acceptor does reset its sequence-number store (correct), but its Logon response does not carry 141=Y. The counterparty is therefore never told the sequence numbers were reset, which breaks the bilateral sequence-number contract on the next exchange (the acceptor expects 34=2 / sends 34=1, while the peer still tracks its old counters → resend/gap-fill churn or disconnect).

The acceptor's reply only ever echoes the inbound 141 flag; it never reflects its own ResetOnLogon setting.

Expected behavior

Per the FIX session protocol, a sequence-number reset must be acknowledged with 141=Y. So when the acceptor resets its store (because ResetOnLogon=Y), its Logon response should set 141=Y — regardless of whether the initiator requested the reset.

This is also what the initiator path already does (see below), so the two roles are inconsistent today.

Actual behavior

The acceptor resets the store but replies without 141=Y unless the initiator's Logon happened to carry 141=Y.

Root cause

In session.go, handleLogon (acceptor branch) computes a local resetStore that correctly captures whether a reset occurred:

// handleLogon
resetStore = s.ResetOnLogon            // acceptor honors the config
...
var resetSeqNumFlag FIXBoolean
if err := msg.Body.GetField(tagResetSeqNumFlag, &resetSeqNumFlag); err == nil {
    if resetSeqNumFlag && !s.sentReset {
        resetStore = true              // inbound flag also forces a reset
    }
}
if resetStore {
    s.store.Reset()                    // <-- the reset actually happens
}
...
// but the REPLY only passes the *inbound* flag, not resetStore:
if err := s.sendLogonInReplyTo(resetSeqNumFlag.Bool(), msg); err != nil { ... }

sendLogonInReplyTo then sets 141 only when that argument is true:

func (s *session) sendLogonInReplyTo(setResetSeqNum bool, inReplyTo *Message) error {
    ...
    if setResetSeqNum {
        logon.Body.SetField(tagResetSeqNumFlag, FIXBoolean(true))
    }
    ...
}

By contrast, the initiator path does fold the config in via shouldSendReset():

func (s *session) sendLogon() error {
    return s.sendLogonInReplyTo(s.shouldSendReset(), nil)
}

func (s *session) shouldSendReset() bool {
    ...
    return (s.ResetOnLogon || s.ResetOnDisconnect || s.ResetOnLogout) &&
        s.store.NextTargetMsgSeqNum() == 1 && s.store.NextSenderMsgSeqNum() == 1
}

So ResetOnLogon reaches the wire for an initiator's Logon, but is dropped from an acceptor's Logon reply.

Steps to reproduce

1. Run a quickfix-go acceptor with ResetOnLogon=Y:

   [DEFAULT]
   ConnectionType=acceptor
   ResetOnLogon=Y
   
   [SESSION]
   BeginString=FIX.4.4
   SenderCompID=SERVER
   TargetCompID=CLIENT
   SocketAcceptPort=5001

2. Connect any initiator and send a Logon without 141 (i.e. omit ResetSeqNumFlag entirely)

3. Observe the acceptor's Logon response (35=A): it has no tag 141, even though the acceptor reset its store (confirm via the "resetting sequence numbers to 1" event / store state, and via the 34=1 it sends).

4. Now repeat sending 141=Y in the initiator's Logon → the response does
   contain 141=Y. The only thing that changed is the inbound flag, not the
   acceptor's reset behavior.

Note on test coverage

logon_state_test.go:TestFixMsgInLogonResetSeqNum sets tagResetSeqNumFlag on the inbound Logon, so it passes regardless of this bug. There is no test for the ResetOnLogon=Y + inbound-without-141 case, which is exactly the gap.

Suggested fix

Pass the already-computed resetStore (whether a reset actually happened) to the reply, instead of only the inbound flag:

-   if err := s.sendLogonInReplyTo(resetSeqNumFlag.Bool(), msg); err != nil {
+   if err := s.sendLogonInReplyTo(resetStore, msg); err != nil {

This advertises 141=Y exactly when the acceptor reset its sequence numbers, mirroring the initiator's shouldSendReset() behavior. (prepMessageForSend already re-reads tagResetSeqNumFlag after ToAdmin, so the on-wire/store state stays consistent; the resulting reset is idempotent since handleLogon already reset the store.)

Workaround

Applications can set the flag in Application.ToAdmin for outbound Logon messages on reset-on-logon sessions (prepMessageForSend honors the mutation and keeps the store reset consistent).