TimelordUK / jspurefix

native typescript FIX engine
MIT License
58 stars 27 forks source link

Tag length #14

Closed NiklasZ closed 3 years ago

NiklasZ commented 3 years ago

Hi,

A while ago I ran into an issue when interacting with a 3rd party (closed source) engine, where it could not understand any generated FIX 4.2 message (using the library repo), because the BodyLength(9) tag was too long. So for example it couldn't read the logon message:

8=FIX.4.2|9=0000069|35=A|49=redacted|56=redacted|34=1|52=20210108-08:08:45.297|98=0|108=30|141=Y|10=187|

Because it expected the BodyLength to only be up to 6 digits instead of 7 (9=000069 vs. 9=0000069). Neither their engine nor jspurefix had an easy config-level way to tweak this (from what I can tell, but maybe I'm wrong), so I forked and modified the jspurefix engine: https://github.com/TimelordUK/jspurefix/compare/master...NiklasZ:1.0.17-fixup.

As I'm still not that familiar with other FIX engines I have some questions:

  1. Is this kind of problem common?
  2. Would there have been a better way to do this with the existing jspurefix implementation?
  3. Is it worthwhile to make something like this configurable?

Feedback is appreciated :slightly_smiling_face:

TimelordUK commented 3 years ago

hi,

thanks for raising this

I have added to master a change allowing the width to be configured with the "BodyLengthChars": 6

Will release shortly, testing the addition of tls socket for SSL based clients where certificate is required.

Regarding issues of this type, generally the rule is the server is right and hence we as a client have to send what they want.

I had wondered if this would eventually be raised, thanks

2021-02-13T19:14:27.313Z [test_client:FixSession] info: [0] 8 (BeginString) = FIX4.4, [1] 9 (BodyLength) = 000112 [2] 35 (MsgType) = AQ[TradeCaptureReportRequestAck], [3] 49 (SenderCompID) = accept-tls-comp [4] 56 (TargetCompID) = init-tls-comp, [5] 34 (MsgSeqNum) = 8 [6] 57 (TargetSubID) = fix, [7] 52 (SendingTime) = 20210213-19:14:27.294 [8] 568 (TradeRequestID) = all-trades, [9] 569 (TradeRequestType) = 0[AllTrades] [10] 749 (TradeRequestResult) = 0[Successful], [11] 750 (TradeRequestStatus) = 1[Completed] [12] 10 (CheckSum) = 147

{ "application": { "reconnectSeconds": 10, "type": "initiator", "name": "test_client", "tcp": { "host" : "localhost", "port": 2344, "tls": { "timeout": 10000, "sessionTimeout": 10000, "enableTrace": true, "key": "data/session/certs/client/client.key", "cert": "data/session/certs/client/client.crt", "ca": [ "data/session/certs/ca/ca.crt" ] } }, "protocol": "ascii", "dictionary": "repo44" }, "Username": "js-tls-client", "Password": "pwd-tls-client", "EncryptMethod": 0, "ResetSeqNumFlag": true, "HeartBtInt": 30, "SenderCompId": "init-tls-comp", "TargetCompID": "accept-tls-comp", "TargetSubID": "fix", "BeginString": "FIX4.4", "BodyLengthChars": 6 }

On Thu, 11 Feb 2021 at 10:45, NiklasZ notifications@github.com wrote:

Hi,

A while ago I ran into an issue when interacting with a 3rd party (closed source) engine, where it could not understand any generated FIX 4.2 message (using the library repo), because the BodyLength(9) tag was too long. So for example it couldn't read the logon message:

8=FIX.4.2|9=0000069|35=A|49=redacted|56=redacted|34=1|52=20210108-08:08:45.297|98=0|108=30|141=Y|10=187|

Because it expected the BodyLength to only be up to 6 digits instead of 7 ( 9=000069 vs. 9=0000069). Neither their engine nor jspurefix had an easy config-level way to tweak this (from what I can tell, but maybe I'm wrong), so I forked and modified the jspurefix engine: master...NiklasZ:1.0.17-fixup https://github.com/TimelordUK/jspurefix/compare/master...NiklasZ:1.0.17-fixup .

As I'm still not that familiar with other FIX engines I have some questions:

  1. Is this kind of problem common?
  2. Would there have been a better way to do this with the existing jspurefix implementation?
  3. Is it worthwhile to make something like this configurable?

Feedback is appreciated 🙂

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TimelordUK/jspurefix/issues/14, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXWJG4EUH2BWXLUANV2DF3S6OYLZANCNFSM4XOTEUBQ .

NiklasZ commented 3 years ago

Thanks for adding this to the configuration :pray: . I tested "jspurefix": "1.1.1" just now and can confirm the new BodyLengthChars property is working for me!

On a sidenote, I'm not sure if tls makes sense as a required property in

export interface ITcpTransportDescription {
  readonly port: number
  readonly host: string,
  readonly tls: ITlsOptions
}

as it's not actually needed to send messages. That said, it can easily be ignored via

tls: undefined as any, // non-production messages are not encrypted

so it's not much of a concern.

TimelordUK commented 3 years ago

you are quite right it should not be specified required - thanks for pointing that out - I shall correct it.

On Tue, 23 Feb 2021 at 17:34, NiklasZ notifications@github.com wrote:

Thanks for adding this to the configuration 🙏 . I tested "jspurefix": "1.1.1" just now and can confirm the new BodyLengthChars property is working for me!

On a sidenote, I'm not sure if tls makes sense as a required property in

export interface ITcpTransportDescription {

readonly port: number

readonly host: string,

readonly tls: ITlsOptions }

as it's not actually needed to send messages. That said, it can easily be ignored via

tls: undefined as any, // non-production messages are not encrypted

so it's not much of a concern.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/TimelordUK/jspurefix/issues/14#issuecomment-784374513, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXWJGY7MH4WAOZR6GF5DOLTAPRLBANCNFSM4XOTEUBQ .