connamara / quickfixn

QuickFIX/n implements the FIX protocol on .NET.
http://quickfixn.org
Other
476 stars 562 forks source link

issue with Hop groups in allocation instruction J #313

Open XavierDt opened 9 years ago

XavierDt commented 9 years ago

Hi,

when receiving the hereunder allocation instruction (parties were renamed so length and hash are no longer consistent) on our quickfix server, an exception was thrown (see hereunder complete stack) , due to a bad index search in a dictionary. Exception is thrown from file DataDictionary\DataDictionary.cs in method

public void Iterate(FieldMap map, string msgType), line 213

Faulty Code section is :

// check contents of each group
foreach (int groupTag in map.GetGroupTags())
{
    for (int i = 1; i <= map.GroupCount(groupTag); i++)
    {
        Group g = map.GetGroup(i, groupTag);
        DDGrp ddg = this.Messages[msgType].GetGroup(groupTag);
        // throw an exception if key not found
        IterateGroup(g, ddg, msgType)
    }
}

This loop is looking for groupTag 627 in Messages["J"] and this throws the exception. Adding a condition

if (Messages[msgType].Groups.ContainsKey(groupTag)) 

before the GetGroup() line solves the issue, but i wonder if it might hide a deeper issue ?

regards

Xavier

Exception stack :

   at System.ThrowHelper.ThrowKeyNotFoundException()
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at QuickFix.DataDictionary.DDMap.GetGroup(Int32 tag) in c:\HOMEWARE\quickfixn-master\QuickFIXn\DataDictionary\DDMap.cs:line 39
   at QuickFix.DataDictionary.DataDictionary.Iterate(FieldMap map, String msgType) in c:\HOMEWARE\quickfixn-master\QuickFIXn\DataDictionary\DataDictionary.cs:line 213
   at QuickFix.DataDictionary.DataDictionary.Validate(Message message, DataDictionary sessionDataDict, DataDictionary appDataDict, String beginString, String msgType) in c:\HOMEWARE\quickfixn-master\QuickFIXn\DataDictionary\DataDictionary.cs:line 102
   at QuickFix.DataDictionary.DataDictionary.Validate(Message message, Boolean bodyOnly, String beginString, String msgType) in c:\HOMEWARE\quickfixn-master\QuickFIXn\DataDictionary\DataDictionary.cs:line 126
   at QuickFix.DataDictionary.DataDictionary.Validate(Message message, String beginString, String msgType) in c:\HOMEWARE\quickfixn-master\QuickFIXn\DataDictionary\DataDictionary.cs:line 111
   at QuickFix.Session.Next(Message message) in c:\HOMEWARE\quickfixn-master\QuickFIXn\Session.cs:line 581
   at QuickFix.Session.Next(String msgStr) in c:\HOMEWARE\quickfixn-master\QuickFIXn\Session.cs:line 518
   at QuickFix.SocketReader.OnMessageFoundInternal(String msg) in c:\HOMEWARE\quickfixn-master\QuickFIXn\SocketReader.cs:line 150

FIX message :

[8=FIX.4.4|9=598|35=J|56=USER1|115=USER2|50=USER2|627=1|628=ATR|629=20150428-19:23:42|630=2624|34=1869|49=TRANSPORT|52=20150428-19:23:43|70=1358804370|71=0|626=1|857=0|54=2|55=FHLMC_38-79-LT|48=US3137ACNA31|22=4|167=CMO|541=20410115|225=20110601|223=2.5|107=FHLMC_38-79LT|53=12940000|423=1|6=100.00000000|15=USD|453=2|448=USER3|447=D|452=13|448=LEI123456|447=D|452=1|75=20150428|60=20150428-00:00:00.000|64=20150501|381=2509555.91|118=2509555.91|157=0|235=CURRENT|236=0.000000|78=1|79=TST-TRD3|80=12940000|467=3574-4696|12=0.00|13=3|154=2509555.91|742=0.00|136=1|137=0.00|138=USD|139=7|891=0|10=073]
gbirchmeier commented 9 years ago

Did the thrown exception crash the engine? That would definitely be a bug.

XavierDt commented 9 years ago

Hello, the session doesn't crash, but the message itself is lost (the exception is thrown before any user call back is called). regards Xavier

gbirchmeier commented 9 years ago

Here's a unit test that reproduces the issue (added to DataDictionaryTests.cs).

I think this is related to the NoHops group in the header. It's the only repeating group that is defined in the StandardHeader, and there is no test that includes this field. It may be entirely possible that nobody has ever used QF/n to receive a message that includes NoHops! (Seriously, who uses NoHops? I don't even know what it's for.)

Here's a test that reproduces your issue (added to DataDictionaryTests.cs).

    [Test] // Issue #313 investigation
    public void Issue313()
    {
        QuickFix.DataDictionary.DataDictionary dd = new QuickFix.DataDictionary.DataDictionary("../../../spec/fix/FIX44.xml");
        QuickFix.FIX44.MessageFactory f = new QuickFix.FIX44.MessageFactory();

        string msgStr = "8=FIX.4.4|9=575|35=J|56=USER1|115=USER2|50=USER2|627=1|628=ATR|629=20150428-19:23:42|630=2624|34=1869|49=TRANSPORT|52=20150428-19:23:43|70=1358804370|71=0|626=1|857=0|54=2|55=FHLMC_38-79-LT|48=US3137ACNA31|22=4|167=CMO|541=20410115|225=20110601|223=2.5|107=FHLMC_38-79LT|53=12940000|423=1|6=100.00000000|15=USD|453=2|448=USER3|447=D|452=13|448=LEI123456|447=D|452=1|75=20150428|60=20150428-00:00:00.000|64=20150501|381=2509555.91|118=2509555.91|157=0|235=CURRENT|236=0.000000|78=1|79=TST-TRD3|80=12940000|467=3574-4696|12=0.00|13=3|154=2509555.91|742=0.00|136=1|137=0.00|138=USD|139=7|891=0|10=089|".Replace("|", Message.SOH);

        string msgType = "J";
        string beginString = "FIX.4.4";

        Message message = f.Create(beginString, msgType);
        message.FromString(msgStr, true, dd, dd);

        Assert.DoesNotThrow(delegate { dd.Validate(message, beginString, msgType); });
    }

I think I'm on the way to a fix.

itaydemri commented 9 years ago

Hello, I encountered the same issue now, however the session does crash.

the exception triggers a disconnect:

catch (System.Exception e)
{
    if (null != session_)
        session_.Disconnect(e.ToString());
    else
        Disconnect();
    }

Regards, Itay

gbirchmeier commented 9 years ago

Yes, it's well understood now, I just need to make time to fix it.

gbirchmeier commented 5 years ago

It chokes on this line in DataDictionary.cs Iterate(map,msgType):

// check contents of each group
foreach (int groupTag in map.GetGroupTags())
{
    for (int i = 1; i <= map.GroupCount(groupTag); i++)
    {
        Group g = map.GetGroup(i, groupTag);
---->           DDGrp ddg = this.Messages[msgType].GetGroup(groupTag);
        IterateGroup(g, ddg, msgType);
    }
}

In that line, msgType=J and groupTag=627 (NoHops).

The choke is inside the GetGroup() call. The parser doesn't realize it's still in a header, and it's looking inside the J msg definition and expecting to find the NoHops group definition. Of course it's not there.

To do this totally correctly, we should probably switch to a generated Header class, rather than this janky hardcoded one that defines no fields and has no typesafe methods. I'm not really excited about writing that, because I'll have to generate separate headers for FIX41,FIX42,etc and I'm worried about side effects.

Could be easy, could be a rabbit hole.