QuickFIX/J

ValidateFieldsOutOfOrder=N not applied on all message levels

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.4.0
  • Fix Version/s: 1.5.1
  • Component/s: Engine
  • Labels:
    None
  • Environment:
    All

Description

QuickFIX configuration flag ValidateFieldsOutOfOrder=N has been applied. Although QuickFIX/J should ignore field ordering on all repeating group levels (as specified by ValidateFieldsOutOfOrder=N), some session rejects from the engine have been observed by the clients and in internal test, which reject FIX messages because of improper field ordering (example: reversed order of PartyIDSource (447) and PartyRole (452) in the Parties group). After reviewing the QuickFIX/J source code, it has been established that the cause of the problem is that the flag is not propagated properly to second and higher groups levels in the recursive validation of the FIX message.

Fixed: propagate the flag properly.

    public void setCheckFieldsHaveValues(boolean flag) {
        checkFieldsHaveValues = flag;
        for(GroupInfo gi : groups.values()) { // ******* ADDED *******
            gi.getDataDictionary().setCheckFieldsHaveValues(flag);
        }
    }

(Internal reference: IC_58606)

Issue Links

Activity

Hide
Steve Bate added a comment - 22/Apr/11 12:56 AM

This is a misunderstanding of the documented purpose of the ValidateFieldsOutOfOrder flag: "If set to N, fields that are out of order (i.e. body fields in the header, or header fields in the body) will not be rejected.". It is not related at all to group field ordering or and field ordering other than specific header fields. This option was inherited from the QuickFIX C++ implementation.

Show
Steve Bate added a comment - 22/Apr/11 12:56 AM This is a misunderstanding of the documented purpose of the ValidateFieldsOutOfOrder flag: "If set to N, fields that are out of order (i.e. body fields in the header, or header fields in the body) will not be rejected.". It is not related at all to group field ordering or and field ordering other than specific header fields. This option was inherited from the QuickFIX C++ implementation.
Hide
Steve Bate added a comment - 22/Apr/11 12:59 AM

To be more precise, it allows headers fields in the body and vice versa. So it's not actually related to specific header fields, but any header/body incorrect orderings. Again, it was not intended to inhibit validation of group field ordering.

Show
Steve Bate added a comment - 22/Apr/11 12:59 AM To be more precise, it allows headers fields in the body and vice versa. So it's not actually related to specific header fields, but any header/body incorrect orderings. Again, it was not intended to inhibit validation of group field ordering.
Hide
Grant Birchmeier added a comment - 24/Apr/11 10:08 PM

Steve, I agree with your assessment. Is there a reason you didn't close this bug?

Show
Grant Birchmeier added a comment - 24/Apr/11 10:08 PM Steve, I agree with your assessment. Is there a reason you didn't close this bug?
Hide
Steve Bate added a comment - 25/Apr/11 6:50 PM

Eric checked in some changes related to this issue (SVN Rev #1019). I was wondering if he had a different interpretation of the issue.

Show
Steve Bate added a comment - 25/Apr/11 6:50 PM Eric checked in some changes related to this issue (SVN Rev #1019). I was wondering if he had a different interpretation of the issue.
Hide
Eric Deshayes added a comment - 26/Apr/11 3:40 PM

Steve, we fixed the issue in our integration branch by keeping the actually implemented semantic: we check the fields orders as it is done in quickfix.Message.parseGroup(String, StringField, DataDictionary, FieldMap).
The problem was that it was properly done in first level group but not in the groups of a group.

We could add the semantic you are referring to (out of order meaning body fields in the header, or header fields in the body) if needed. If we want to have both this semantic and the current one, we would have to add a new boolean setting.

Show
Eric Deshayes added a comment - 26/Apr/11 3:40 PM Steve, we fixed the issue in our integration branch by keeping the actually implemented semantic: we check the fields orders as it is done in quickfix.Message.parseGroup(String, StringField, DataDictionary, FieldMap). The problem was that it was properly done in first level group but not in the groups of a group. We could add the semantic you are referring to (out of order meaning body fields in the header, or header fields in the body) if needed. If we want to have both this semantic and the current one, we would have to add a new boolean setting.
Hide
Grant Birchmeier added a comment - 26/Apr/11 4:01 PM

Is such a feature really needed? I've had counterparties in the past who've put fields out of order, but the resulting order was always deterministic. Simply rearranging the DataDictionary was all that was needed.

Show
Grant Birchmeier added a comment - 26/Apr/11 4:01 PM Is such a feature really needed? I've had counterparties in the past who've put fields out of order, but the resulting order was always deterministic. Simply rearranging the DataDictionary was all that was needed.
Hide
Christoph John added a comment - 05/May/11 5:52 PM

Hi guys,

I'm a little confused. Could anyone please tell me what is wrong with ValidateFieldsOutOfOrder? Is the documentation wrong or the code? There are some tickets dealing with this, e.g. QFJ-477 and QFJ-520 (dup).

Our situation: We have a customer which suddenly reported that his messages get rejected after we have switched from QuickFIX/J 1.3.1 to 1.5.0. Looking at the messages we could see that he sent tag 50 (SenderSubID) incorrectly at the end of the message. On the old and the new version we have set ValidateFieldsOutOfOrder=N.
So my assumption would be that both QuickFIX versions behave the same. But with 1.5.0 the messages get rejected.

Looking at the documentation (and also stated by Steve in the second comment: "To be more precise, it allows headers fields in the body and vice versa. So it's not actually related to specific header fields, but any header/body incorrect orderings.") I would suppose that the message could be parsed correctly with either version. But with 1.5.0 this fails.

Here is a short test which fails with 1.5.0 regardless if I set dataDictionary.setCheckFieldsOutOfOrder to true or false.

{{ @Test
public void testNewOrderSingleWithMisplacedTag50()
throws Exception {

final DataDictionary dataDictionary = new DataDictionary( "etc/FIX.4.2.xml" );

// behaviour for QuickFIX 1.3.1: true -> parsing of nos2 fails
// behaviour for QuickFIX 1.3.1: false -> parsing of nos2 succeeds
// behaviour for QuickFIX 1.5.0: true -> parsing of nos2 fails
// behaviour for QuickFIX 1.5.0: false -> parsing of nos2 fails
dataDictionary.setCheckFieldsOutOfOrder( false );

String correctFixMessage = "8=FIX.4.2|9=218|35=D|49=cust|50=trader|56=FixGateway|34=449|52=20110420-09:17:40|11=clordid|54=1|38=50|59=6|40=2|44=77.1|"
+ "432=20110531|15=CHF|22=8|55=symbol|48=CH1234.CHF|21=1|60=20110420-11:17:39.000|63=0|207=VX|10=007|";
correctFixMessage = correctFixMessage.replace( '|', '\001' );
final NewOrderSingle nos = new NewOrderSingle();
nos.fromString( correctFixMessage, dataDictionary, true );
dataDictionary.validate( nos );
assertTrue( nos.getHeader().isSetField( new SenderSubID() ) );

String incorrectFixMessage = "8=FIX.4.2|9=218|35=D|49=cust|56=FixGateway|34=449|52=20110420-09:17:40|11=clordid|54=1|38=50|59=6|40=2|44=77.1|"
+ "432=20110531|15=CHF|22=8|55=symbol|48=CH1234.CHF|21=1|60=20110420-11:17:39.000|63=0|207=VX|50=trader|10=007|";
incorrectFixMessage = incorrectFixMessage.replace( '|', '\001' );
final NewOrderSingle nos2 = new NewOrderSingle();
nos2.fromString( incorrectFixMessage, dataDictionary, true );
dataDictionary.validate( nos2 );
assertTrue( nos2.getHeader().isSetField( new SenderSubID() ) ); }}

Any help is greatly appreciated.

Thanks
Chris.

Show
Christoph John added a comment - 05/May/11 5:52 PM Hi guys, I'm a little confused. Could anyone please tell me what is wrong with ValidateFieldsOutOfOrder? Is the documentation wrong or the code? There are some tickets dealing with this, e.g. QFJ-477 and QFJ-520 (dup). Our situation: We have a customer which suddenly reported that his messages get rejected after we have switched from QuickFIX/J 1.3.1 to 1.5.0. Looking at the messages we could see that he sent tag 50 (SenderSubID) incorrectly at the end of the message. On the old and the new version we have set ValidateFieldsOutOfOrder=N. So my assumption would be that both QuickFIX versions behave the same. But with 1.5.0 the messages get rejected. Looking at the documentation (and also stated by Steve in the second comment: "To be more precise, it allows headers fields in the body and vice versa. So it's not actually related to specific header fields, but any header/body incorrect orderings.") I would suppose that the message could be parsed correctly with either version. But with 1.5.0 this fails. Here is a short test which fails with 1.5.0 regardless if I set dataDictionary.setCheckFieldsOutOfOrder to true or false. {{ @Test public void testNewOrderSingleWithMisplacedTag50() throws Exception { final DataDictionary dataDictionary = new DataDictionary( "etc/FIX.4.2.xml" ); // behaviour for QuickFIX 1.3.1: true -> parsing of nos2 fails // behaviour for QuickFIX 1.3.1: false -> parsing of nos2 succeeds // behaviour for QuickFIX 1.5.0: true -> parsing of nos2 fails // behaviour for QuickFIX 1.5.0: false -> parsing of nos2 fails dataDictionary.setCheckFieldsOutOfOrder( false ); String correctFixMessage = "8=FIX.4.2|9=218|35=D|49=cust|50=trader|56=FixGateway|34=449|52=20110420-09:17:40|11=clordid|54=1|38=50|59=6|40=2|44=77.1|" + "432=20110531|15=CHF|22=8|55=symbol|48=CH1234.CHF|21=1|60=20110420-11:17:39.000|63=0|207=VX|10=007|"; correctFixMessage = correctFixMessage.replace( '|', '\001' ); final NewOrderSingle nos = new NewOrderSingle(); nos.fromString( correctFixMessage, dataDictionary, true ); dataDictionary.validate( nos ); assertTrue( nos.getHeader().isSetField( new SenderSubID() ) ); String incorrectFixMessage = "8=FIX.4.2|9=218|35=D|49=cust|56=FixGateway|34=449|52=20110420-09:17:40|11=clordid|54=1|38=50|59=6|40=2|44=77.1|" + "432=20110531|15=CHF|22=8|55=symbol|48=CH1234.CHF|21=1|60=20110420-11:17:39.000|63=0|207=VX|50=trader|10=007|"; incorrectFixMessage = incorrectFixMessage.replace( '|', '\001' ); final NewOrderSingle nos2 = new NewOrderSingle(); nos2.fromString( incorrectFixMessage, dataDictionary, true ); dataDictionary.validate( nos2 ); assertTrue( nos2.getHeader().isSetField( new SenderSubID() ) ); }} Any help is greatly appreciated. Thanks Chris.
Hide
Jörg Thönnes added a comment - 10/May/11 10:04 AM

In reply to comment #5:
> We could add the semantic you are referring to (out of order meaning body fields
> in the header, or header fields in the body) if needed. If we want to have both
> this semantic and the current one, we would have to add a new boolean setting.

Salut Eric,

could you re-add this semantic for the QF/J 1.5.1 release as you offered above?
We rely on it and it caused us major troubles with one customer.

Merci, Jörg

Show
Jörg Thönnes added a comment - 10/May/11 10:04 AM In reply to comment #5: > We could add the semantic you are referring to (out of order meaning body fields > in the header, or header fields in the body) if needed. If we want to have both > this semantic and the current one, we would have to add a new boolean setting. Salut Eric, could you re-add this semantic for the QF/J 1.5.1 release as you offered above? We rely on it and it caused us major troubles with one customer. Merci, Jörg
Hide
Eric Deshayes added a comment - 12/May/11 8:48 AM

Hi Jörg,
I will fix the behaviour for the ValidateFieldsOutOfOrder property as per the documentation (and in sync with quickfix c++): out of order meaning body fields in the header, or header fields in the body.
I will add a new property to bypass the check on the group fields order.

Show
Eric Deshayes added a comment - 12/May/11 8:48 AM Hi Jörg, I will fix the behaviour for the ValidateFieldsOutOfOrder property as per the documentation (and in sync with quickfix c++): out of order meaning body fields in the header, or header fields in the body. I will add a new property to bypass the check on the group fields order.
Hide
Eric Deshayes added a comment - 12/May/11 12:30 PM

committed on the integration branch in rev#1032

added new session property to ignore misordered group fields: ValidateUnorderedGroupFields.

for the initial ValidateFieldsOutOfOrder property, the expected behaviour has been implemented.
added 2 unit tests :
quickfix.DataDictionaryTest.testNewOrderSingleWithCorrectTag50()
quickfix.DataDictionaryTest.testNewOrderSingleWithMisplacedTag50()

Show
Eric Deshayes added a comment - 12/May/11 12:30 PM committed on the integration branch in rev#1032 added new session property to ignore misordered group fields: ValidateUnorderedGroupFields. for the initial ValidateFieldsOutOfOrder property, the expected behaviour has been implemented. added 2 unit tests : quickfix.DataDictionaryTest.testNewOrderSingleWithCorrectTag50() quickfix.DataDictionaryTest.testNewOrderSingleWithMisplacedTag50()

People

  • Assignee:
    Unassigned
    Reporter:
    SSE
Vote (2)
Watch (3)

Dates

  • Created:
    28/Jun/10 2:52 PM
    Updated:
    20/May/11 2:16 PM
    Resolved:
    20/May/11 2:16 PM