Issue with empty values in parser

[hsnaves]

Hi,

I noticed one issue with the parser, and would like to submit a patch. I could not attach the patch file to this message (GitHub does not allow the patch extension), so I am sending it below.

From: Humberto Naves <hsnaves@gmail.com>
Date: Thu, 3 Feb 2022 12:52:57 -0500
Subject: [PATCH] Parser now accepts empty string as the value in the tag/value
 pair.

Previously the parser would parse the FIX message "TAG1=|TAG2=VAL|" as a
single pair with tag "TAG1" and corresponding value "|TAG2=VAL", which
is incorrect. It should instead parse this message into two pairs:
("TAG1", ""), and ("TAG2", "VAL"). This commit fixes this issue.

Note that the character `|` is being used as a representation of the
EOH byte in the message above.
---
 simplefix/parser.py |  3 +++
 test/test_parser.py | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/simplefix/parser.py b/simplefix/parser.py
index 2d59ffa..32e0ae7 100644
--- a/simplefix/parser.py
+++ b/simplefix/parser.py
@@ -192,6 +192,9 @@ class FixParser(object):  # skipcq: PYL-R0205
                     in_tag = False
                     start = point

+                # To avoid skipping a pair when the value is empty
+                point -= 1
+
             elif c == self.stop_char:
                 if start != point:
                     value = self.buf[start:point]
diff --git a/test/test_parser.py b/test/test_parser.py
index 775b069..1977392 100644
--- a/test/test_parser.py
+++ b/test/test_parser.py
@@ -317,6 +317,19 @@ class ParserTests(unittest.TestCase):
         self.assertIsNotNone(m)
         self.assertEqual(b"3", m.get(34))

+    def test_empty_value(self):
+        """Test empty value in message."""
+
+        buf = b'8=FIX.4.2\x019=9\x0135=D\x0129=\x0110=098\x01'
+
+        p = FixParser()
+        p.append_buffer(buf)
+
+        m = p.get_message()
+        self.assertIsNotNone(m)
+        self.assertEqual(b"D", m.get(35))
+        self.assertEqual(b"", m.get(29))
+

 if __name__ == "__main__":
     unittest.main()
-- 
2.35.1

[da4089]

Hello, and thank you for the issue report and especially the patch!

This is clearly an error in the parser. It certainly shouldn't be taking the next tag=value text as part of the value here. But I am a little uncertain about just applying your patch, because I had a quick look at the FIX standards, and they say

"All tags must have a value specified. Optional fields without values should simply not be specified in the FIX message. A Reject message is the appropriate response to a tag with no value." -- FIX 4.4 with Errata 20030618, page 18.

OTOH, I imagine that you discovered this problem because something is sending you a FIX message with a zero-length value?

I'll think about this over the weekend.

[hsnaves]

OTOH, I imagine that you discovered this problem because something is sending you a FIX message with a zero-length value?

Yes, I discovered this issue while parsing some malformed FIX messages. I wish all vendors would adhere to the standards, but unfortunately it is not the case.

Do you think a possible compromise/solution could be to parse the message anyway, but issue a warning? Or maybe even better: change the signature of the get_message() method to something like get_message(self, pedantic=True), with an extra parameter that tells the method what to do when it encounters such situations? If the message does not strictly adhere to the standard, and pedantic=True, the method can then issue an exception.

[da4089]

I've pushed a change that will support this, by configuring the parser.

Instead of eg. p = FixParser(), you should now do p = FixParser(allow_empty_values=True)

I'll update the doc and push a release to PyPI in a day or so.

[da4089]

Fixed in release v1.0.15 and doc updated.

Thanks again for the report!

[hsnaves]

Thank you!