Parsing an invalid FIX message could cause infinite loop. (#434)

Corrected `FIXMessageDecoder`
quickfix-j · Nov 10, 2021 · d034e71 · d034e71
1 parent b69cac3
commit d034e71
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 2 deletions.
diff --git a/quickfixj-core/src/main/java/quickfix/mina/message/FIXMessageDecoder.java b/quickfixj-core/src/main/java/quickfix/mina/message/FIXMessageDecoder.java
@@ -170,8 +170,8 @@ private boolean parseMessage(IoBuffer in, ProtocolDecoderOutput out)
                     } else {
                         if (position < in.limit()) { // if data remains
                             String messageString = getMessageStringForError(in);
-                            handleError(in, in.position() + 1, "Length format error in message (last character: " + (char)ch + "): " + messageString,
-                                false);
+                            handleError(in, position, "Length format error in message (last character: " + (char) ch + "): " + messageString,
+                                    false);
                             continue;
                         } else {
                             break;

diff --git a/quickfixj-core/src/test/java/quickfix/mina/message/FIXMessageDecoderTest.java b/quickfixj-core/src/test/java/quickfix/mina/message/FIXMessageDecoderTest.java
@@ -490,4 +490,30 @@ public void testBadBodyLength() throws Exception {
         setUpBuffer(message);
         assertMessageFound(goodMessage);
     }
+
+    /**
+     * Several bad messages after each other should not send the decoder in an
+     * infinite loop. https://github.com/quickfix-j/quickfixj/issues/432
+     */
+    @Test(timeout = 1000)
+    public void testLengthFormatError() throws Exception {
+        String badMessages = "8=FIX.4.4\0019=058=\0018=FIX.4.4\0019=058=\0018=FIX.4.4\0019=058=\0018=FIX.4.4\0019=058=\001";
+        String goodMessage = "8=FIX.4.4\0019=12\00135=Y\001108=30\00110=037\001";
+        setUpBuffer(badMessages + goodMessage + badMessages + goodMessage);
+        assertMessageFound(goodMessage, 2);
+    }
+
+    /**
+     * Several bad messages after each other should not send the decoder in an
+     * infinite loop. https://github.com/quickfix-j/quickfixj/issues/432
+     */
+    @Test(timeout = 1000)
+    public void testLengthFormatError2() throws Exception {
+        decoder = new FIXMessageDecoder("UTF-16");
+        setUpBuffer("8=FIX.4.2\0019=128=FIX.4.2\0019=8=FIX.4.2\0019=128="
+                + "FIX.4.2\0019=8=FIX.4.2\0019=12\00135=X\001108=30\00110=049\001");
+        MessageDecoderResult decoderResult = decoder.decode(null, buffer, decoderOutput);
+        assertEquals("wrong decoder result", MessageDecoderResult.OK, decoderResult);
+        assertEquals("Wrong encoding", 14397, (int) decoderOutput.getMessage().charAt(0));
+    }
 }