Merge pull request #57 from the-thing/allowUnknownMsgFieldsFix

QFJ-656 - fixed logic around field validation (cherry picked from commit fbafe6b)
quickfix-j · Jan 6, 2016 · 13b081b · 13b081b
1 parent 43c3f0c
commit 13b081b
Show file tree

Hide file tree

Showing 4 changed files with 331 additions and 27 deletions.
diff --git a/quickfixj-core/src/main/doc/usermanual/usage/configuration.html b/quickfixj-core/src/main/doc/usermanual/usage/configuration.html
@@ -330,7 +330,7 @@ <H3>QuickFIX Settings</H3>
   </TR>
   <TR ALIGN="left" VALIGN="middle">
     <TD><I>ValidateUserDefinedFields</I></TD>
-    <TD>If set to N, user defined fields will not be rejected if they are not
+    <TD>If set to N, user defined fields (field with tag >= 5000) will not be rejected if they are not
       defined in the data dictionary, or are present in messages they do not
       belong to.</TD>
     <TD>Y<br>
@@ -369,8 +369,9 @@ <H3>QuickFIX Settings</H3>
   </TR>
   <TR ALIGN="left" VALIGN="middle">
     <TD><I>AllowUnknownMsgFields</I></TD>
-    <TD>Allow unknown fields in messages. This is intended for unknown fields with tags < 5000
-     (not user defined fields). Use ValidateUserDefinedFields for controlling validation of tags >= 5000.</TD>
+    <TD>If set to Y, non user defined fields (field with tag < 5000) will not be rejected if they are not
+      defined in the data dictionary, or are present in messages they do not
+      belong to.
     <TD>Y<br>
       N</TD>
     <TD>N</TD>

diff --git a/quickfixj-core/src/main/java/quickfix/DataDictionary.java b/quickfixj-core/src/main/java/quickfix/DataDictionary.java
@@ -654,11 +654,8 @@ private void iterate(FieldMap map, String msgType, DataDictionary dd) throws Inc
                 checkValue(field);
             }
 
-            if (beginString != null && shouldCheckTag(field)) {
-                dd.checkValidTagNumber(field);
-                if (map instanceof Message) {
-                    checkIsInMessage(field, msgType);
-                }
+            if (beginString != null) {
+                dd.checkField(field, msgType, map instanceof Message);
                 dd.checkGroupCount(field, map, msgType);
             }
         }
@@ -680,17 +677,33 @@ private void checkMsgType(String msgType) {
         }
     }
 
-    // / If we need to check for the tag in the dictionary
-    private boolean shouldCheckTag(Field<?> field) {
-        return checkUserDefinedFields || field.getField() < USER_DEFINED_TAG_MIN;
-    }
-
     // / Check if field tag number is defined in spec.
     void checkValidTagNumber(Field<?> field) {
         if (!fields.contains(Integer.valueOf(field.getTag()))) {
             throw new FieldException(SessionRejectReason.INVALID_TAG_NUMBER, field.getField());
         }
     }
+
+    // / Check if field tag is defined for message or group
+    void checkField(Field<?> field, String msgType, boolean message) {
+        // use different validation for groups and messages
+        boolean messageField = message ? isMsgField(msgType, field.getField()) : fields.contains(field.getField());
+        boolean fail;
+
+        if (field.getField() < USER_DEFINED_TAG_MIN) {
+            fail = !messageField && !allowUnknownMessageFields; 
+        } else {
+            fail = !messageField && checkUserDefinedFields; 
+        }
+
+        if (fail) {
+            if (fields.contains(Integer.valueOf(field.getTag()))) {
+                throw new FieldException(SessionRejectReason.TAG_NOT_DEFINED_FOR_THIS_MESSAGE_TYPE, field.getField());
+            } else {
+                throw new FieldException(SessionRejectReason.INVALID_TAG_NUMBER, field.getField());
+            }
+        }
+    }
 
     private void checkValidFormat(StringField field) throws IncorrectDataFormat {
         try {
@@ -773,14 +786,6 @@ private void checkHasValue(StringField field) {
         }
     }
 
-    // / Check if a field is in this message type.
-    private void checkIsInMessage(Field<?> field, String msgType) {
-        if (!isMsgField(msgType, field.getField()) && !allowUnknownMessageFields) {
-            throw new FieldException(SessionRejectReason.TAG_NOT_DEFINED_FOR_THIS_MESSAGE_TYPE,
-                    field.getField());
-        }
-    }
-
     // / Check if group count matches number of groups in
     private void checkGroupCount(StringField field, FieldMap fieldMap, String msgType) {
         final int fieldNum = field.getField();