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

issue with Hop groups in allocation instruction J #313

Open
XavierDt opened this issue May 19, 2015 · 6 comments
Open

issue with Hop groups in allocation instruction J #313

XavierDt opened this issue May 19, 2015 · 6 comments
Assignees

Comments

@XavierDt
Copy link

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
Copy link
Member

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

@XavierDt
Copy link
Author

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
Copy link
Member

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
Copy link

itaydemri commented Sep 10, 2015

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
Copy link
Member

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

gbirchmeier added a commit to gbirchmeier/quickfixn that referenced this issue Sep 11, 2015
@gbirchmeier gbirchmeier self-assigned this Sep 16, 2015
@gbirchmeier
Copy link
Member

gbirchmeier commented Jul 13, 2019

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.

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

3 participants