[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: | qfj169.patch | ||||||||||||||||||||||||
Issue Links: |
|
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. Then try to "roundtrip" (ie do new Message(origMessage.toString(), dictionary)) and create a new message from the old one. 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. 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. 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 |
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 ] |
|