Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order of SignatureLength, Signature in StandardTrailer #692

Open
vanrein opened this issue Feb 15, 2025 · 1 comment
Open

Order of SignatureLength, Signature in StandardTrailer #692

vanrein opened this issue Feb 15, 2025 · 1 comment

Comments

@vanrein
Copy link

vanrein commented Feb 15, 2025

Had to get into QuickFIX/Go, but it's growing on me. Thanks!

I'm building a FIX application using OpenPGP-signed FIXT messages.
This revealed what I found to be a bug in the order of StandardTrailer fields, as coded in

quickfix/message.go

Lines 87 to 97 in 2ed31c3

// In the trailer, CheckSum (tag 10) must be last.
func trailerFieldOrdering(i, j Tag) bool {
switch {
case i == tagCheckSum:
return false
case j == tagCheckSum:
return true
}
return i < j
}

This implicitly places Signature(89) before SignatureLength(93), instead of the intended order,

  • SignatureLength(93) if used at all
  • Signature(89) if used at all
  • Checksum(10)

Placing SignatureLength after Signature impairs customary front-to-back parsing.

Since these are the only three fields in the StandardTrailer component, reverse ordering of tag values would fix the problem (until a fourth field is added perhaps).

Minimal code to demonstrate this bug:

package main

import "fmt"
import qfapi "github.com/quickfixgo/quickfix"

func main () {
        msg := qfapi.NewMessage();
        msg.Trailer.SetField (93, qfapi.FIXInt (5))
        msg.Trailer.SetField (89, qfapi.FIXString ("VROEM"))
        fmt.Print (msg, "\r\n")
}

It does not help to manually map to bytes to have msg.cook() invoked.
The order of the msg.Trailer.SetField() calls also has no impact.
All those non-workarounds make sense, having seen the trailerFieldOrdering() implementation.

Let me know if you'd like a PR, but I'm new to QuickFIX and don't default to it.

@vanrein
Copy link
Author

vanrein commented Feb 15, 2025

FYI, I tested the reverse tag ordering and it works-for-me, and probably for anyone else too:
https://gitlab.com/arpa2/quickfixgo/-/commit/a6c125ece626a74a06b1a51b49924627fa3de68c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant