[QFJ-169] Message parsing fails on messages with invalid fields in repeating groups with non-obvious errors Created: 28/Apr/07  Updated: 27/Jul/17  Resolved: 05/Apr/17

Status: Closed
Project: QuickFIX/J
Component/s: Engine
Affects Version/s: 1.1.0
Fix Version/s: 1.6.4

Type: Bug Priority: Default
Reporter: Toli Kuznets Assignee: Christoph John
Resolution: Fixed Votes: 2
Labels: None

Attachments: Text File qfj169.patch    
Issue Links:
Duplicate
is duplicated by QFJ-269 validate method in DataDictionary doe... Closed
is duplicated by QFJ-909 Unexpected Behaviour for ValidateInco... Closed
Relates
relates to QFJ-168 Add repeating group validation in mes... Closed
is related to QFJ-791 An unexpected field in a repeating gr... Closed

 Description   

When you create a Message from incoming string that has invalid fields in the repeating group, the message parsing code will fail with a non-obvious (and not helpful) error. This works when you have 2 or more groups and never on just 1 group.

Create a message and add a group to it, and add an invalid field to the first group.
Add group to the message
Add another (valid or invalid) group to the message.

Then try to "roundtrip" (ie do new Message(origMessage.toString(), dictionary)) and create a new message from the old one.
The creation will fail, and the error you get is "Tag not defined for this message" - but the tag you get is the first tag after the invalid one in the repeating group, not the actual tag that's invalid.

What happens is that in Message.parseGroup() function when are in a repeating group, when we find the first invalid field we essentially "pushback" the invalid field and get out of the group parse, skipping any other group fields.

another Bug: we report back the initially calculated group count, not the # of groups we actually parsed

We then add the pushed-back field to the new message, and try to inspect the next field - which belongs in the repeating group and not in message itself.
So the parser throws an exception in that case, saying that field is invalid, while it really should've given an error about the previous field.

See the attached patch for unit test.

The fix is non-trivial - you have to know whether the repeating groups are over and you just got the next "real message" field, or if it's just an invalid field for the repeating group.
Also, you can't just check if the pushed-back field doesn't belong to the parent message, since it may, but you may still have repeating groups coming up.

Perhaps the best way to fix it is to check if the "offending" field is a field in the parent's message type, and if it's not then report an error. You'd do it before pushing the field back. May need to pass the parent's message type through to parseGroup, in case we have groups nested inside other groups.



 Comments   
Comment by Toli Kuznets [ 28/Apr/07 ]

validation may catch this earlier

Comment by Toli Kuznets [ 02/May/07 ]

Patch containing the unit test exhibiting the failure
patch in core/ directory

Comment by Toli Kuznets [ 02/May/07 ]

Removed the code from the bug and added it as a patch attachments, should make the bug cleaner and easier to read.

Comment by Christoph John [ 05/Apr/17 ]
  • extended Message.parseGroup() to accept unknown fields in repeating groups
    • if validation is disabled (either completely or depending on ValidateUserDefinedFields or AllowUnknownMsgFields)
    • if the field encountered is not a trailer or message field
  • moreover, if validation is enabled, the incorrect field will be reported (not the next field that is encountered)
  • added unit tests from QFJ issues
  • removed synchronization from FieldException in Message since it seemed unnecessary
Generated at Sat Nov 23 05:35:34 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.