Uploaded image for project: 'QuickFIX/J'
  1. QuickFIX/J
  2. QFJ-169

Message parsing fails on messages with invalid fields in repeating groups with non-obvious errors

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Default
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.6.4
    • Component/s: Engine
    • Labels:
      None

      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.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                chrjohn Christoph John
                Reporter:
                toli Toli Kuznets
              • Votes:
                2 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: