Merge branch 'main' into dependabot/go_modules/golang.org/x/crypto-0.17.0

ackleymi · web-flow · commit ab0414597358 · 2024-04-22T20:08:31.000-05:00
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -21,15 +21,14 @@ jobs:
     name: Linter
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-go@v4
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
         with:
           go-version: '1.21'
-          cache: false
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v4
         with:
-          version: v1.51
+          version: v1.57.2
 
   build:
     name: build
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,6 +1,12 @@
 run:
   timeout: 10m
-  skip-dirs:
+  # deprecated config - 
+  # skip-dirs:
+  #   - gen
+  #   - vendor
+
+issues:
+  exclude-dirs:
     - gen
     - vendor
 
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 0.9.1 (April 15, 2024)
+* Preserve original body when resending https://github.com/quickfixgo/quickfix/pull/624
+
 ## 0.9.0 (November 13, 2023)
 
 ### FEATURES
diff --git a/Makefile b/Makefile
@@ -19,7 +19,7 @@ test:
 linters-install:
 	@golangci-lint --version >/dev/null 2>&1 || { \
 		echo "installing linting tools..."; \
-		curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v1.50.1; \
+		curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v1.57.2; \
 	}
 
 lint: linters-install
diff --git a/accepter_test.go b/accepter_test.go
@@ -21,7 +21,7 @@ import (
 
 	"github.com/quickfixgo/quickfix/config"
 
-	proxyproto "github.com/armon/go-proxyproto"
+	proxyproto "github.com/pires/go-proxyproto"
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/acceptor.go b/acceptor.go
@@ -25,7 +25,7 @@ import (
 	"strconv"
 	"sync"
 
-	proxyproto "github.com/armon/go-proxyproto"
+	proxyproto "github.com/pires/go-proxyproto"
 
 	"github.com/quickfixgo/quickfix/config"
 )
diff --git a/datadictionary/datadictionary.go b/datadictionary/datadictionary.go
@@ -315,7 +315,7 @@ func Parse(path string) (*DataDictionary, error) {
 func ParseSrc(xmlSrc io.Reader) (*DataDictionary, error) {
 	doc := new(XMLDoc)
 	decoder := xml.NewDecoder(xmlSrc)
-	decoder.CharsetReader = func(encoding string, input io.Reader) (io.Reader, error) {
+	decoder.CharsetReader = func(_ string, input io.Reader) (io.Reader, error) {
 		return input, nil
 	}
 
diff --git a/go.mod b/go.mod
@@ -5,6 +5,7 @@ go 1.21
 require (
 	github.com/armon/go-proxyproto v0.1.0
 	github.com/mattn/go-sqlite3 v1.14.18
+	github.com/pires/go-proxyproto v0.7.0
 	github.com/pkg/errors v0.9.1
 	github.com/shopspring/decimal v1.3.1
 	github.com/stretchr/testify v1.8.4
diff --git a/go.sum b/go.sum
@@ -23,6 +23,8 @@ github.com/mattn/go-sqlite3 v1.14.18/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S
 github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc=
 github.com/montanaflynn/stats v0.6.6 h1:Duep6KMIDpY4Yo11iFsvyqJDyfzLF9+sndUKT+v64GQ=
 github.com/montanaflynn/stats v0.6.6/go.mod h1:etXPPgVO6n31NxCd9KQUMvCM+ve0ruNzt6R8Bnaayow=
+github.com/pires/go-proxyproto v0.7.0 h1:IukmRewDQFWC7kfnb66CSomk2q/seBuilHBYFwyq0Hs=
+github.com/pires/go-proxyproto v0.7.0/go.mod h1:Vz/1JPY/OACxWGQNIRY2BeyDmpoaWmEP40O9LbuiFR4=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
diff --git a/in_session.go b/in_session.go
@@ -253,7 +253,7 @@ func (state inSession) resendMessages(session *session, beginSeqNo, endSeqNo int
 		}
 
 		session.log.OnEventf("Resending Message: %v", sentMessageSeqNum)
-		msgBytes = msg.build()
+		msgBytes = msg.buildWithBodyBytes(msg.bodyBytes) // workaround for maintaining repeating group field order
 		session.EnqueueBytesAndSend(msgBytes)
 
 		seqNum = sentMessageSeqNum + 1
diff --git a/logon_state.go b/logon_state.go
@@ -68,7 +68,7 @@ func (s logonState) Timeout(session *session, e internal.Event) (nextState sessi
 	return s
 }
 
-func (s logonState) Stop(session *session) (nextState sessionState) {
+func (s logonState) Stop(_ *session) (nextState sessionState) {
 	return latentState{}
 }
 
diff --git a/logon_state_test.go b/logon_state_test.go
@@ -333,7 +333,7 @@ func (s *LogonStateTestSuite) TestFixMsgInLogonSeqNumTooHigh() {
 	s.Require().Nil(err)
 	s.MessageType(string(msgTypeLogon), sentMessage)
 
-	s.session.sendQueued()
+	s.session.sendQueued(true)
 	s.MessageType(string(msgTypeResendRequest), s.MockApp.lastToAdmin)
 	s.FieldEquals(tagBeginSeqNo, 1, s.MockApp.lastToAdmin.Body)
 
@@ -373,7 +373,7 @@ func (s *LogonStateTestSuite) TestFixMsgInLogonSeqNumTooLow() {
 	s.Require().Nil(err)
 	s.MessageType(string(msgTypeLogout), sentMessage)
 
-	s.session.sendQueued()
+	s.session.sendQueued(true)
 	s.MessageType(string(msgTypeLogout), s.MockApp.lastToAdmin)
 	s.FieldEquals(tagText, "MsgSeqNum too low, expecting 2 but received 1", s.MockApp.lastToAdmin.Body)
 }
diff --git a/logout_state.go b/logout_state.go
@@ -40,6 +40,6 @@ func (state logoutState) Timeout(session *session, event internal.Event) (nextSt
 	return state
 }
 
-func (state logoutState) Stop(session *session) (nextstate sessionState) {
+func (state logoutState) Stop(_ *session) (nextstate sessionState) {
 	return state
 }
diff --git a/memorystore.go b/memorystore.go
@@ -109,7 +109,7 @@ func (store *memoryStore) GetMessages(beginSeqNum, endSeqNum int) ([][]byte, err
 
 type memoryStoreFactory struct{}
 
-func (f memoryStoreFactory) Create(sessionID SessionID) (MessageStore, error) {
+func (f memoryStoreFactory) Create(_ SessionID) (MessageStore, error) {
 	m := new(memoryStore)
 	if err := m.Reset(); err != nil {
 		return m, errors.Wrap(err, "reset")
diff --git a/message.go b/message.go
@@ -152,7 +152,7 @@ func ParseMessageWithDataDictionary(
 	msg *Message,
 	rawMessage *bytes.Buffer,
 	transportDataDictionary *datadictionary.DataDictionary,
-	applicationDataDictionary *datadictionary.DataDictionary,
+	_ *datadictionary.DataDictionary,
 ) (err error) {
 	msg.Header.Clear()
 	msg.Body.Clear()
@@ -210,6 +210,7 @@ func ParseMessageWithDataDictionary(
 
 	trailerBytes := []byte{}
 	foundBody := false
+	foundTrailer := false
 	for {
 		parsedFieldBytes = &msg.fields[fieldIndex]
 		if xmlDataLen > 0 {
@@ -228,6 +229,7 @@ func ParseMessageWithDataDictionary(
 			msg.Header.add(msg.fields[fieldIndex : fieldIndex+1])
 		case isTrailerField(parsedFieldBytes.tag, transportDataDictionary):
 			msg.Trailer.add(msg.fields[fieldIndex : fieldIndex+1])
+			foundTrailer = true
 		default:
 			foundBody = true
 			trailerBytes = rawBytes
@@ -247,6 +249,12 @@ func ParseMessageWithDataDictionary(
 		fieldIndex++
 	}
 
+	// This will happen if there are no fields in the body
+	if foundTrailer && !foundBody {
+		trailerBytes = rawBytes
+		msg.bodyBytes = nil
+	}
+
 	// Body length would only be larger than trailer if fields out of order.
 	if len(msg.bodyBytes) > len(trailerBytes) {
 		msg.bodyBytes = msg.bodyBytes[:len(msg.bodyBytes)-len(trailerBytes)]
@@ -314,7 +322,7 @@ func (m *Message) IsMsgTypeOf(msgType string) bool {
 func (m *Message) reverseRoute() *Message {
 	reverseMsg := NewMessage()
 
-	copy := func(src Tag, dest Tag) {
+	copyFunc := func(src Tag, dest Tag) {
 		var field FIXString
 		if m.Header.GetField(src, &field) == nil {
 			if len(field) != 0 {
@@ -323,25 +331,25 @@ func (m *Message) reverseRoute() *Message {
 		}
 	}
 
-	copy(tagSenderCompID, tagTargetCompID)
-	copy(tagSenderSubID, tagTargetSubID)
-	copy(tagSenderLocationID, tagTargetLocationID)
+	copyFunc(tagSenderCompID, tagTargetCompID)
+	copyFunc(tagSenderSubID, tagTargetSubID)
+	copyFunc(tagSenderLocationID, tagTargetLocationID)
 
-	copy(tagTargetCompID, tagSenderCompID)
-	copy(tagTargetSubID, tagSenderSubID)
-	copy(tagTargetLocationID, tagSenderLocationID)
+	copyFunc(tagTargetCompID, tagSenderCompID)
+	copyFunc(tagTargetSubID, tagSenderSubID)
+	copyFunc(tagTargetLocationID, tagSenderLocationID)
 
-	copy(tagOnBehalfOfCompID, tagDeliverToCompID)
-	copy(tagOnBehalfOfSubID, tagDeliverToSubID)
-	copy(tagDeliverToCompID, tagOnBehalfOfCompID)
-	copy(tagDeliverToSubID, tagOnBehalfOfSubID)
+	copyFunc(tagOnBehalfOfCompID, tagDeliverToCompID)
+	copyFunc(tagOnBehalfOfSubID, tagDeliverToSubID)
+	copyFunc(tagDeliverToCompID, tagOnBehalfOfCompID)
+	copyFunc(tagDeliverToSubID, tagOnBehalfOfSubID)
 
 	// Tags added in 4.1.
 	var beginString FIXString
 	if m.Header.GetField(tagBeginString, &beginString) == nil {
 		if string(beginString) != BeginStringFIX40 {
-			copy(tagOnBehalfOfLocationID, tagDeliverToLocationID)
-			copy(tagDeliverToLocationID, tagOnBehalfOfLocationID)
+			copyFunc(tagOnBehalfOfLocationID, tagDeliverToLocationID)
+			copyFunc(tagDeliverToLocationID, tagOnBehalfOfLocationID)
 		}
 	}
 
@@ -417,6 +425,21 @@ func (m *Message) build() []byte {
 	return b.Bytes()
 }
 
+// Constructs a []byte from a Message instance, using the given bodyBytes.
+// This is a workaround for the fact that we currently rely on the generated Message types to properly serialize/deserialize RepeatingGroups.
+// In other words, we cannot go from bytes to a Message then back to bytes, which is exactly what we need to do in the case of a Resend.
+// This func lets us pull the Message from the Store, parse it, update the Header, and then build it back into bytes using the original Body.
+// Note: The only standard non-Body group is NoHops.  If that is used in the Header, this workaround may fail.
+func (m *Message) buildWithBodyBytes(bodyBytes []byte) []byte {
+	m.cook()
+
+	var b bytes.Buffer
+	m.Header.write(&b)
+	b.Write(bodyBytes)
+	m.Trailer.write(&b)
+	return b.Bytes()
+}
+
 func (m *Message) cook() {
 	bodyLength := m.Header.length() + m.Body.length() + m.Trailer.length()
 	m.Header.SetInt(tagBodyLength, bodyLength)
diff --git a/message_test.go b/message_test.go
@@ -133,18 +133,54 @@ func (s *MessageSuite) TestReBuild() {
 
 	s.msg.Header.SetField(tagOrigSendingTime, FIXString("20140515-19:49:56.659"))
 	s.msg.Header.SetField(tagSendingTime, FIXString("20140615-19:49:56"))
+	s.msg.Header.SetField(tagPossDupFlag, FIXBoolean(true))
 
 	rebuildBytes := s.msg.build()
 
-	expectedBytes := []byte("8=FIX.4.29=12635=D34=249=TW52=20140615-19:49:5656=ISLD122=20140515-19:49:56.65911=10021=140=154=155=TSLA60=00010101-00:00:00.00010=128")
+	expectedBytes := []byte("8=FIX.4.29=13135=D34=243=Y49=TW52=20140615-19:49:5656=ISLD122=20140515-19:49:56.65911=10021=140=154=155=TSLA60=00010101-00:00:00.00010=122")
 
-	s.True(bytes.Equal(expectedBytes, rebuildBytes), "Unexpected bytes,\n +%s\n-%s", rebuildBytes, expectedBytes)
+	s.True(bytes.Equal(expectedBytes, rebuildBytes), "Unexpected bytes,\n +%s\n -%s", rebuildBytes, expectedBytes)
 
 	expectedBodyBytes := []byte("11=10021=140=154=155=TSLA60=00010101-00:00:00.000")
 
 	s.True(bytes.Equal(s.msg.bodyBytes, expectedBodyBytes), "Incorrect body bytes, got %s", string(s.msg.bodyBytes))
 }
 
+func (s *MessageSuite) TestReBuildWithRepeatingGroupForResend() {
+	// Given the following message with a repeating group
+	origHeader := "8=FIXT.1.19=16135=834=349=ISLD52=20240415-03:43:17.92356=TW"
+	origBody := "6=1.0011=114=1.0017=131=1.0032=1.0037=138=1.0039=254=155=1150=2151=0.00453=1448=xyzzy447=D452=1"
+	origTrailer := "10=014"
+	rawMsg := bytes.NewBufferString(origHeader + origBody + origTrailer)
+
+	// When I reparse the message from the store during a resend request
+	s.Nil(ParseMessage(s.msg, rawMsg))
+
+	// And I update the headers for resend
+	s.msg.Header.SetField(tagOrigSendingTime, FIXString("20240415-03:43:17.923"))
+	s.msg.Header.SetField(tagSendingTime, FIXString("20240415-14:41:23.456"))
+	s.msg.Header.SetField(tagPossDupFlag, FIXBoolean(true))
+
+	// When I rebuild the message
+	rebuildBytes := s.msg.build()
+
+	// Then the repeating groups will not be in the correct order in the rebuilt message (note tags 447, 448, 452, 453)
+	expectedBytes := []byte("8=FIXT.1.19=19235=834=343=Y49=ISLD52=20240415-14:41:23.45656=TW122=20240415-03:43:17.9236=1.0011=114=1.0017=131=1.0032=1.0037=138=1.0039=254=155=1150=2151=0.00453=1448=xyzzy447=D452=110=018")
+	s.False(bytes.Equal(expectedBytes, rebuildBytes), "Unexpected bytes,\n expected: %s\n  but was: %s", expectedBytes, rebuildBytes)
+	expectedOutOfOrderBytes := []byte("8=FIXT.1.19=19235=834=343=Y49=ISLD52=20240415-14:41:23.45656=TW122=20240415-03:43:17.9236=1.0011=114=1.0017=131=1.0032=1.0037=138=1.0039=254=155=1150=2151=0.00447=D448=xyzzy452=1453=110=018")
+	s.True(bytes.Equal(expectedOutOfOrderBytes, rebuildBytes), "Unexpected bytes,\n expected: %s\n  but was: %s", expectedOutOfOrderBytes, rebuildBytes)
+
+	// But the bodyBytes will still be correct
+	origBodyBytes := []byte(origBody)
+	s.True(bytes.Equal(origBodyBytes, s.msg.bodyBytes), "Incorrect body bytes, \n expected: %s\n  but was: %s", origBodyBytes, s.msg.bodyBytes)
+
+	// So when I combine the updated header + the original bodyBytes + the as-is trailer
+	resendBytes := s.msg.buildWithBodyBytes(s.msg.bodyBytes)
+
+	// Then the reparsed, rebuilt message will retain the correct ordering of repeating group tags during resend
+	s.True(bytes.Equal(expectedBytes, resendBytes), "Unexpected bytes,\n expected: %s\n  but was: %s", expectedBytes, resendBytes)
+}
+
 func (s *MessageSuite) TestReverseRoute() {
 	s.Nil(ParseMessage(s.msg, bytes.NewBufferString("8=FIX.4.29=17135=D34=249=TW50=KK52=20060102-15:04:0556=ISLD57=AP144=BB115=JCD116=CS128=MG129=CB142=JV143=RY145=BH11=ID21=338=10040=w54=155=INTC60=20060102-15:04:0510=123")))
 
diff --git a/null_log.go b/null_log.go
@@ -17,17 +17,17 @@ package quickfix
 
 type nullLog struct{}
 
-func (l nullLog) OnIncoming([]byte)                        {}
-func (l nullLog) OnOutgoing([]byte)                        {}
-func (l nullLog) OnEvent(string)                           {}
-func (l nullLog) OnEventf(format string, a ...interface{}) {}
+func (l nullLog) OnIncoming([]byte)                   {}
+func (l nullLog) OnOutgoing([]byte)                   {}
+func (l nullLog) OnEvent(string)                      {}
+func (l nullLog) OnEventf(_ string, _ ...interface{}) {}
 
 type nullLogFactory struct{}
 
 func (nullLogFactory) Create() (Log, error) {
 	return nullLog{}, nil
 }
-func (nullLogFactory) CreateSessionLog(sessionID SessionID) (Log, error) {
+func (nullLogFactory) CreateSessionLog(_ SessionID) (Log, error) {
 	return nullLog{}, nil
 }
 
diff --git a/quickfix_test.go b/quickfix_test.go
@@ -85,26 +85,26 @@ type MockApp struct {
 	lastToApp       *Message
 }
 
-func (e *MockApp) OnCreate(sessionID SessionID) {
+func (e *MockApp) OnCreate(_ SessionID) {
 }
 
-func (e *MockApp) OnLogon(sessionID SessionID) {
+func (e *MockApp) OnLogon(_ SessionID) {
 	e.Called()
 }
 
-func (e *MockApp) OnLogout(sessionID SessionID) {
+func (e *MockApp) OnLogout(_ SessionID) {
 	e.Called()
 }
 
-func (e *MockApp) FromAdmin(msg *Message, sessionID SessionID) (reject MessageRejectError) {
+func (e *MockApp) FromAdmin(_ *Message, _ SessionID) (reject MessageRejectError) {
 	if err, ok := e.Called().Get(0).(MessageRejectError); ok {
 		return err
 	}
 
 	return nil
 }
 
-func (e *MockApp) ToAdmin(msg *Message, sessionID SessionID) {
+func (e *MockApp) ToAdmin(msg *Message, _ SessionID) {
 	e.Called()
 
 	if e.decorateToAdmin != nil {
@@ -114,12 +114,12 @@ func (e *MockApp) ToAdmin(msg *Message, sessionID SessionID) {
 	e.lastToAdmin = msg
 }
 
-func (e *MockApp) ToApp(msg *Message, sessionID SessionID) (err error) {
+func (e *MockApp) ToApp(msg *Message, _ SessionID) (err error) {
 	e.lastToApp = msg
 	return e.Called().Error(0)
 }
 
-func (e *MockApp) FromApp(msg *Message, sessionID SessionID) (reject MessageRejectError) {
+func (e *MockApp) FromApp(_ *Message, _ SessionID) (reject MessageRejectError) {
 	if err, ok := e.Called().Get(0).(MessageRejectError); ok {
 		return err
 	}
diff --git a/session.go b/session.go
diff --git a/session_factory_test.go b/session_factory_test.go
diff --git a/session_state.go b/session_state.go
diff --git a/store/sql/sqlstore.go b/store/sql/sqlstore.go
diff --git a/validation.go b/validation.go
diff --git a/validation_test.go b/validation_test.go

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ test:`
`19`	`19`	`linters-install:`
`20`	`20`	`@golangci-lint --version >/dev/null 2>&1 \|\| { \`
`21`	`21`	`echo "installing linting tools..."; \`
`22`		`- curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh\| sh -s v1.50.1; \`
	`22`	`+ curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh\| sh -s v1.57.2; \`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`lint: linters-install`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ import (`
`21`	`21`
`22`	`22`	`"github.com/quickfixgo/quickfix/config"`
`23`	`23`
`24`		`- proxyproto "github.com/armon/go-proxyproto"`
	`24`	`+ proxyproto "github.com/pires/go-proxyproto"`
`25`	`25`	`"github.com/stretchr/testify/assert"`
`26`	`26`	`)`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ import (`
`25`	`25`	`"strconv"`
`26`	`26`	`"sync"`
`27`	`27`
`28`		`- proxyproto "github.com/armon/go-proxyproto"`
	`28`	`+ proxyproto "github.com/pires/go-proxyproto"`
`29`	`29`
`30`	`30`	`"github.com/quickfixgo/quickfix/config"`
`31`	`31`	`)`
Original file line number	Diff line number	Diff line change
`@@ -315,7 +315,7 @@ func Parse(path string) (*DataDictionary, error) {`
`315`	`315`	`func ParseSrc(xmlSrc io.Reader) (*DataDictionary, error) {`
`316`	`316`	`doc := new(XMLDoc)`
`317`	`317`	`decoder := xml.NewDecoder(xmlSrc)`
`318`		`- decoder.CharsetReader = func(encoding string, input io.Reader) (io.Reader, error) {`
	`318`	`+ decoder.CharsetReader = func(_ string, input io.Reader) (io.Reader, error) {`
`319`	`319`	`return input, nil`
`320`	`320`	`}`
`321`	`321`
Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ func (state inSession) resendMessages(session *session, beginSeqNo, endSeqNo int`
`253`	`253`	`}`
`254`	`254`
`255`	`255`	`session.log.OnEventf("Resending Message: %v", sentMessageSeqNum)`
`256`		`- msgBytes = msg.build()`
	`256`	`+ msgBytes = msg.buildWithBodyBytes(msg.bodyBytes) // workaround for maintaining repeating group field order`
`257`	`257`	`session.EnqueueBytesAndSend(msgBytes)`
`258`	`258`
`259`	`259`	`seqNum = sentMessageSeqNum + 1`