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

Proper handling of length defined Xml Data Fields #782

Closed

Conversation

larsope
Copy link
Contributor

@larsope larsope commented Jun 26, 2023

use XmlDataLen field to parse XmlData in FIX messages rather than looking for delimiters to end field value

  • Added ExctractDataField method that is called when the parser comes across a FIX data field that has a defined length given by a previous tag.
  • Added a unit test to DataDictionaryTests to verify that a message is
    read correctly

- use XmlDataLen field to parse XmdData in FIX messags rather than looking for delimiters
- added test
@gbirchmeier
Copy link
Member

Was the existing behavior broken in some way?

I think this is probably a good change, but how about you spell it out for me.

@larsope
Copy link
Contributor Author

larsope commented Jul 18, 2023

@gbirchmeier Sorry for the delay. The existing behavior is incorrect when the XmlData field contains an "=" in the block. An example of this scenario is a CME DropCopy FIX message similar to the one that I added in the test. In these messages, the CME uses the XmlData field to wrap the original FIX execution report that it is relaying.

https://www.cmegroup.com/confluence/display/EPICSANDBOX/Drop+Copy+4.0#:~:text=XMLData%20(213)%20field.-,213,iLink%20or%20processed%20FIX%20message%2C%20embedded%20in%20an%20XML%20field%20%E2%80%9CRTRF%E2%80%9D.,-10

The existing QuickFix/n message parsing logic determines the end of the value associated with any tag by looking for the index of the next "=" character.

This issue has been addressed in the C++ version of QuickFix in a similar manner as the one I have proposed. (see line 625 of https://github.com/quickfix/quickfix/blob/master/src/C%2B%2B/Message.cpp).

Thanks for taking a look at this! I really appreciate it. I have been on a custom build of this repo for a while to get around this issue.

@larsope
Copy link
Contributor Author

larsope commented Nov 1, 2023

@gbirchmeier just following up. Do you have any thoughts on this? I have been using this fork in production for 3 years now without any issues.

@dominicpalmer
Copy link

Hi @gbirchmeier, please than this PR be considered for merge? As @larsope mentioned, QuickFIX/n is currently unable to parse FIX with XmlData that embeds an execution report. Hacks or forks are the only way around this at the moment.

gbirchmeier pushed a commit to gbirchmeier/quickfixn that referenced this pull request Feb 12, 2024
- use XmlDataLen field to parse XmdData in FIX messags rather than looking for delimiters
- added test

pr connamara#782
gbirchmeier pushed a commit to gbirchmeier/quickfixn that referenced this pull request Feb 12, 2024
- use XmlDataLen field to parse XmdData in FIX messags rather than looking for delimiters
- added test

pr connamara#782
gbirchmeier pushed a commit to gbirchmeier/quickfixn that referenced this pull request Feb 12, 2024
- use XmlDataLen field to parse XmdData in FIX messags rather than looking for delimiters
- added test

pr connamara#782
@gbirchmeier
Copy link
Member

I rebased and made some adjustments today. Sorry for not jumping on it sooner*. If you have a little time, please take a look at #832 and confirm that you don't think I messed up anything in the rebase.

(*For what it's worth, I'm trying to be more proactive on this project going forward)

gbirchmeier pushed a commit to gbirchmeier/quickfixn that referenced this pull request Feb 13, 2024
- use XmlDataLen field to parse XmdData in FIX messags rather than looking for delimiters
- added test

pr connamara#782
gbirchmeier added a commit that referenced this pull request Feb 13, 2024
…field_length_parsing

(rebase #782) Proper handling of length defined data fields
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

Successfully merging this pull request may close these issues.

3 participants