Merge branch 'master' of github.com:connamara/quickfixn into issue_255

AcceptanceTest/definitions/server/future/issue146_MissingGroupDelimiter.def → AcceptanceTest/definitions/server/fix44/issue146_MissingGroupDelimiter.def

@@ -1,16 +1,16 @@
-# This is relevant to issue 146
-# Reject message when field is the wrong type -- nested-group case
+# issues 146/270
+# Message has group that does not start with correct delimiter
 
 iCONNECT
 # logon message and response
 I8=FIX.4.4
35=A
34=1
49=TW
52=<TIME>
56=ISLD
98=0
108=2

 E8=FIX.4.4
9=60
35=A
34=1
49=ISLD
52=00000000-00:00:00.000
56=TW
98=0
108=2
10=0

 
-# AP message has group 702, but first element does not start with 703.
+# AP message has group 702, but first element does not start with correct delimiter 703.
 I8=FIX.4.4
35=AP
34=2
49=TW
52=<TIME>
56=ISLD
1=20050500001
55=EURUSD
453=0
581=1
702=1
704=0
705=20000
710=634792896000000000
715=20120802
721=P-DEA30E1PHC0IW7V
730=1.22608
731=1
734=1.22608
753=1
708=20000

 
-# rejected
-E8=FIX.4.4
35=3
34=2
49=ISLD
52=whatever
56=TW
45=2
371=702
372=AP
373=X
58=Group 702's first entry does not start with delimiter 703

+# Rejection resulting from previous message
+E8=FIX.4.4
35=3
34=2
49=ISLD
52=00000000-00:00:00.000
56=TW
45=2
58=Group 702's first entry does not start with delimiter 703
371=702
372=AP
373=99

 
 #Send news and receive echo
 #(basically, force an echo to ensure that above was not rejected)
@@ -20,5 +20,5 @@ E8=FIX.4.4
35=B
34=3
49=ISLD
52=00000000-00:00:00.000
56=TW
33=0
148=FOO

 
 # logout message and response
 I8=FIX.4.4
35=5
34=4
49=TW
52=<TIME>
56=ISLD

-E8=FIX.4.4
9=49
35=4
34=3
49=ISLD
52=00000000-00:00:00.000
56=TW
10=0

+E8=FIX.4.4
9=49
35=5
34=4
49=ISLD
52=00000000-00:00:00.000
56=TW
10=0

 

NEXT_VERSION.md

@@ -37,4 +37,17 @@ Changes since the last version (oldest first):
 * (patch) #278 - fix for mis-sequenced gap fills in re-requested messages (martinadams)
 * (patch) #283 - fix to ensure IApplication.FromAdmin() gets called for received resend requests (martinadams)
 * (minor) #312 - New config option: can force session-level rejects to be resent (martinadams)
+* (minor) #270/#146 - Correct rejection behavior when msg groups do not use correct delimiter tag (gbirchmeier)
+
+
+NOTE: Breaking changes in next release
+---------------------------------------
+This version will have changes that will technically alter the public interface.
+However, most likely no one will be affected.  But just in case, here are the details:
+
+* **class `OtherTagException` was deleted.**  The class `GroupDelimiterTagException` used to
+  be descended from this class, but now descends directly from `TagException`.
+* **method `Session.Next(Message message)` was deleted.**  This should never have been public.
+  If you were using it and this is a problem for you, please contact us
+  via our [Contact Page](http://quickfixn.org/about/contact) and we'll see if we can help.
 

QuickFIXn/Exceptions.cs

@@ -286,20 +286,13 @@ public RepeatingGroupCountMismatch(int tag)
         {}
     }
 
-    public class OtherTagException : TagException
-    {
-        public OtherTagException(string msg, int tag)
-            : base(msg, tag)
-        { }
-    }
-
     /// <summary>
     /// For when a received message has a group that doesn't start its entries with the delimiter tag
     /// </summary>
-    public class GroupDelimiterTagException : OtherTagException
+    public class GroupDelimiterTagException : TagException
     {
         public GroupDelimiterTagException(int counterTag, int delimiterTag)
-            : base(string.Format("Group {0}'s first entry does not start with delimiter {1}", counterTag,delimiterTag), counterTag)
+            : base(string.Format("Group {0}'s first entry does not start with delimiter {1}", counterTag, delimiterTag), counterTag)
         { }
     }
 

QuickFIXn/Message/Message.cs

@@ -368,8 +368,12 @@ public void FromString(string msgstr, bool validate, DataDictionary.DataDictiona
         /// <param name="sessionDD"></param>
         /// <param name="appDD"></param>
         /// <param name="msgFactory">If null, any groups will be constructed as generic Group objects</param>
+        /// <param name="ignoreBody">(default false) if true, ignores all non-header non-trailer fields.
+        ///   Intended for callers that only need rejection-related information from the header.
+        ///   </param>
         public void FromString(string msgstr, bool validate,
-            DataDictionary.DataDictionary sessionDD, DataDictionary.DataDictionary appDD, IMessageFactory msgFactory)
+            DataDictionary.DataDictionary sessionDD, DataDictionary.DataDictionary appDD, IMessageFactory msgFactory,
+            bool ignoreBody=false)
         {
             Clear();
 
@@ -425,7 +429,7 @@ public void FromString(string msgstr, bool validate,
                         pos = SetGroup(f, msgstr, pos, this.Trailer, sessionDD.Trailer.GetGroup(f.Tag), sessionDD, appDD, msgFactory);
                     }
                 }
-                else
+                else if (ignoreBody==false)
                 {
                     if (!expectingBody)
                     {

QuickFIXn/MessageBuilder.cs

@@ -0,0 +1,71 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace QuickFix
+{
+    internal class MessageBuilder
+    {
+        private string _msgStr;
+        private bool _validateLengthAndChecksum;
+        private DataDictionary.DataDictionary _sessionDD;
+        private DataDictionary.DataDictionary _appDD;
+        private IMessageFactory _msgFactory;
+
+        private QuickFix.Fields.MsgType _msgType;
+        private string _beginString;
+
+        private Message _message;
+
+        public string OriginalString { get { return _msgStr; } }
+        public QuickFix.Fields.MsgType MsgType { get { return _msgType; } }
+        public string BeginString { get { return _beginString; } }
+
+        internal MessageBuilder(
+            string msgStr,
+            bool validateLengthAndChecksum,
+            DataDictionary.DataDictionary sessionDD,
+            DataDictionary.DataDictionary appDD,
+            IMessageFactory msgFactory)
+        {
+            _msgStr = msgStr;
+            _validateLengthAndChecksum = validateLengthAndChecksum;
+            _sessionDD = sessionDD;
+            _appDD = appDD;
+            _msgFactory = msgFactory;
+
+            _msgType = Message.IdentifyType(_msgStr);
+            _beginString = Message.ExtractBeginString(_msgStr);
+        }
+
+        internal Message Build()
+        {
+            Message message = _msgFactory.Create(_beginString, _msgType.Obj);
+            message.FromString(
+                _msgStr,
+                _validateLengthAndChecksum,
+                _sessionDD,
+                _appDD,
+                _msgFactory);
+            _message = message;
+            return _message;
+        }
+
+        internal Message RejectableMessage()
+        {
+            if (_message != null)
+                return _message;
+
+            Message message = _msgFactory.Create(_beginString, _msgType.Obj);
+            message.FromString(
+                _msgStr,
+                false,
+                _sessionDD,
+                _appDD,
+                _msgFactory,
+                true);
+            return message;
+        }
+    }
+}

QuickFIXn/QuickFix.csproj

@@ -78,6 +78,7 @@
     <Compile Include="IResponder.cs" />
     <Compile Include="MemoryStore.cs" />
     <Compile Include="MemoryStoreFactory.cs" />
+    <Compile Include="MessageBuilder.cs" />
     <Compile Include="MessageCracker.cs" />
     <Compile Include="Config\Config.cs" />
     <Compile Include="Config\Settings.cs" />
@@ -142,4 +143,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>

QuickFIXn/Session.cs

@@ -508,64 +508,45 @@ public void Next()
         /// <param name="msgStr"></param>
         public void Next(string msgStr)
         {
-            try
-            {
-                this.Log.OnIncoming(msgStr);
-
-                MsgType msgType = Message.IdentifyType(msgStr);
-                string beginString = Message.ExtractBeginString(msgStr);
+            this.Log.OnIncoming(msgStr);
 
-                Message message = msgFactory_.Create(beginString, msgType.Obj);
-                message.FromString(
+            MessageBuilder msgBuilder = new MessageBuilder(
                     msgStr,
                     this.ValidateLengthAndChecksum,
                     this.SessionDataDictionary,
                     this.ApplicationDataDictionary,
                     this.msgFactory_);
 
-                Next(message);
-            }
-            catch (InvalidMessage e)
-            {
-                this.Log.OnEvent(e.Message);
-
-                try
-                {
-                    if (MsgType.LOGON.Equals(Message.IdentifyType(msgStr)))
-                        Disconnect("Logon message is not valid");
-                }
-                catch (MessageParseError)
-                { }
-
-                throw e;
-            }
+            Next(msgBuilder);
         }
 
         /// <summary>
-        /// Process a message from the counterparty. (TODO: consider changing this method to private in v2.0.)
+        /// Process a message from the counterparty.
         /// </summary>
         /// <param name="message"></param>
-        public void Next(Message message)
+        internal void Next(MessageBuilder msgBuilder)
         {
             if (!IsSessionTime)
             {
                 Reset("Out of SessionTime (Session.Next(message))", "Message received outside of session time");
                 return;
             }
 
-            if (appDoesEarlyIntercept_)
-                ((IApplicationExt)Application).FromEarlyIntercept(message, this.SessionID);
-
             if (IsNewSession)
                 state_.Reset("New session (detected in Next(Message))");
 
-            Header header = message.Header;
-            string msgType = "";
+            Message message = null; // declared outside of try-block so that catch-blocks can use it
 
             try
             {
-                msgType = header.GetField(Fields.Tags.MsgType);
-                string beginString = header.GetField(Fields.Tags.BeginString);
+                message = msgBuilder.Build();
+
+                if (appDoesEarlyIntercept_)
+                    ((IApplicationExt)Application).FromEarlyIntercept(message, this.SessionID);
+
+                Header header = message.Header;
+                string msgType = msgBuilder.MsgType.Obj;
+                string beginString = msgBuilder.BeginString;
 
                 if (!beginString.Equals(this.SessionID.BeginString))
                     throw new UnsupportedVersion();
@@ -615,15 +596,29 @@ public void Next(Message message)
                 }
 
             }
+            catch (InvalidMessage e)
+            {
+                this.Log.OnEvent(e.Message);
+
+                try
+                {
+                    if (MsgType.LOGON.Equals(msgBuilder.MsgType.Obj))
+                        Disconnect("Logon message is not valid");
+                }
+                catch (MessageParseError)
+                { }
+
+                throw e;
+            }
             catch (TagException e)
             {
                 if (null != e.InnerException)
                     this.Log.OnEvent(e.InnerException.Message);
-                GenerateReject(message, e.sessionRejectReason, e.Field);
+                GenerateReject(msgBuilder, e.sessionRejectReason, e.Field);
             }
             catch (UnsupportedVersion)
             {
-                if (MsgType.LOGOUT.Equals(msgType))
+                if (MsgType.LOGOUT.Equals(msgBuilder.MsgType.Obj))
                 {
                     NextLogout(message);
                 }
@@ -647,13 +642,13 @@ public void Next(Message message)
                 }
                 else
                 {
-                    if (msgType.Equals(Fields.MsgType.LOGON))
+                    if (MsgType.LOGON.Equals(msgBuilder.MsgType.Obj))
                     {
                         this.Log.OnEvent("Required field missing from logon");
                         Disconnect("Required field missing from logon");
                     }
                     else
-                        GenerateReject(message, new QuickFix.FixValues.SessionRejectReason(SessionRejectReason.REQUIRED_TAG_MISSING, "Required Tag Missing"), e.Field);
+                        GenerateReject(msgBuilder, new QuickFix.FixValues.SessionRejectReason(SessionRejectReason.REQUIRED_TAG_MISSING, "Required Tag Missing"), e.Field);
                 }
             }
             catch (RejectLogon e)
@@ -1348,6 +1343,11 @@ public bool GenerateHeartbeat(Message testRequest)
             return SendRaw(heartbeat, 0);
         }
 
+        internal bool GenerateReject(MessageBuilder msgBuilder, FixValues.SessionRejectReason reason, int field=0)
+        {
+            return GenerateReject(msgBuilder.RejectableMessage(), reason, field);
+        }
+
         public bool GenerateReject(Message message, FixValues.SessionRejectReason reason)
         {
             return GenerateReject(message, reason, 0);

UnitTests/SessionTest.cs

@@ -224,7 +224,7 @@ private void SendLogon(QuickFix.Message msg)
             msg.Header.SetField(new QuickFix.Fields.MsgSeqNum(seqNum++));
             msg.Header.SetField(new QuickFix.Fields.SendingTime(System.DateTime.UtcNow));
             msg.SetField(new QuickFix.Fields.HeartBtInt(1));
-            session.Next(msg);
+            session.Next(msg.ToString());
         }
 
         public bool SENT_RESEND()
@@ -340,7 +340,7 @@ public void SendNOSMessage()
             order.Header.SetField(new QuickFix.Fields.SenderCompID(sessionID.TargetCompID));
             order.Header.SetField(new QuickFix.Fields.MsgSeqNum(seqNum++));
 
-            session.Next(order);
+            session.Next(order.ToString());
         }
 
         public void SendResendRequest(int begin, int end)
@@ -363,7 +363,7 @@ private void SendTheMessage(QuickFix.Message msg)
             msg.Header.SetField(new QuickFix.Fields.SenderCompID(sessionID.TargetCompID));
             msg.Header.SetField(new QuickFix.Fields.MsgSeqNum(seqNum++));
 
-            session.Next(msg);
+            session.Next(msg.ToString());
         }
 
         [Test]
@@ -655,10 +655,10 @@ public void TestMaxMessagesInResendRequest()
 
             reset.Header.SetField(new QuickFix.Fields.MsgSeqNum(2));
             reset.SetField(new QuickFix.Fields.NewSeqNo(2501));
-            session.Next(reset);
+            session.Next(reset.ToString());
 
             order.Header.SetField(new QuickFix.Fields.MsgSeqNum(2501));
-            session.Next(order);
+            session.Next(order.ToString());
 
             // Should have triggered next resend (2502->5001), check this
             Console.WriteLine(responder.msgLookup[QuickFix.Fields.MsgType.RESENDREQUEST].Count);
@@ -670,10 +670,10 @@ public void TestMaxMessagesInResendRequest()
             // Jump forward to the end of the resend chunk with a fillgap reset message
             reset.Header.SetField(new QuickFix.Fields.MsgSeqNum(2502));
             reset.SetField(new QuickFix.Fields.NewSeqNo(5001));
-            session.Next(reset);
+            session.Next(reset.ToString());
 
             order.Header.SetField(new QuickFix.Fields.MsgSeqNum(5001));
-            session.Next(order);   // Triggers next resend (5002->5005)
+            session.Next(order.ToString());   // Triggers next resend (5002->5005)
 
             Console.WriteLine(responder.msgLookup[QuickFix.Fields.MsgType.RESENDREQUEST].Count);
             Assert.That(responder.msgLookup[QuickFix.Fields.MsgType.RESENDREQUEST].Count == 1);
@@ -873,7 +873,7 @@ public void TestApplicationExtension()
             order.Header.SetField(new QuickFix.Fields.SenderCompID(sessionID.TargetCompID));
             order.Header.SetField(new QuickFix.Fields.MsgSeqNum(2));
 
-            session.Next(order);
+            session.Next(order.ToString());
 
             Assert.That(mockApp.InterceptedMessageTypes.Count, Is.EqualTo(2));
             Assert.True(mockApp.InterceptedMessageTypes.Contains(QuickFix.Fields.MsgType.LOGON));