Message Parser and .NET 5.0

[VinceAvery]

The FIX message parser does not work when targeting .NET 5.0 - it fails to parse the message.
This is because the string.IndexOf(string) functionality has changed (see here dotnet/runtime#43736)

The fix was to use to change this line to use StringComparison.Ordinal

quickfixn/QuickFIXn/Message/Message.cs

Line 141 in 63f7e97

int fieldvalend = msgstr.IndexOf((char)1, pos);

From: int fieldvalend = msgstr.IndexOf("\u0001", pos);

To: int fieldvalend = msgstr.IndexOf("\u0001", pos, StringComparison.Ordinal);

[Svisstack]

@VinceAvery It could be related to the #696 Are you able to confirm that after making this change everything works as expected on your side in .NET 5.0 ?

[gbirchmeier]

I have a branch where I have upgraded the engine and acceptance-test apps to .NET 6.0, and everything appears to be working fine.

.NET 5.0 is now EOL, so I am closing this issue.

Please let me know if you think this is a mistake and I will take a second look.

[Svisstack]

@gbirchmeier did you included this change in the .net6.0 version? it looks like it will also be needed in the net6.0. Please also point us where is the branch.

[gbirchmeier]

Sorry, I was not clear:

I did not include this change. I expected the acceptance tests to fail, but they did not. The acceptance test runs a C# test app against a ruby mock QF app, so it does confirm actual transmission/reception of messages on a socket.

The branch is https://github.com/gbirchmeier/quickfixn/tree/net6

Am I missing something? If IndexOf is indeed broken in that branch, please show me how to see the failure so I can make sure I fix it.

[Svisstack]

@gbirchmeier The problem is that the initiator not initiating a connection and do not display any errors; this only occuring on linux (tested on alpine and ubuntu), so when you are running the tests on Windows they will be green. I was not able to attach debugger and look what exceptions are thrown at runtime that's why I'm not sure if the IndexOf is only one issue here.

My assumption was @VinceAvery hit the same bug, fixed and created this issue thread. He didn't mention linux but have same problem that library didnt work after .net5.0 upgrade which should also not work on net6.0

[gbirchmeier]

Ok, that's a start. I've been testing on Windows and OS X. My colleague has a linux dev machine; I'll ask him to check it out next week.

@VinceAvery Can you tell us on which platform you discovered/debugged this error? (Was it Linux?)

(Reopened this issue until we test on Linux and re-evaluate.)

[Svisstack]

@gbirchmeier there is a good practice to run the tests using gitlab actions; build + test and then we can set the image on which this will be run, that way we can test all platforms

[gbirchmeier]

Yes, we know about github actions. After I get this project back into shape we may look into adding that.

[gbirchmeier]

I have merged it to master. Arguably premature, but I'm committed to testing Linux next week. It will be resolved before the next release.

[mgatny]

I cannot reproduce this issue on Ubuntu 22.04 with the dotnet6 pkg installed (which is net6.0). I've done quite a bit of development on qf/n over the past year on that stack with no issue, and we have services deployed using this stack with no issue.

[mgatny]

@gbirchmeier there is a good practice to run the tests using gitlab actions; build + test and then we can set the image on which this will be run, that way we can test all platforms

we set up CI before gh actions existed, so we're currently using appveyor. actions is probably a better solution at this point.

[gbirchmeier]

@VinceAvery @Svisstack Please try the latest master-branch release (now on net6.0) and tell me if you are seeing issues with your respective versions of Linux. As @mgatny says, we are unable to reproduce the issue ourselves.

I am closing this issue for now. If you are still seeing this parser problem with the latest build, please let me know and we will reopen (or open a new issue).

[caoyang1024]

The issue is happening on certain Linux distribution, for our case, it is an .NET 6 application running on AWS linux environment: Amazon Linux 2

root@my_machine-7cfbccd8c6-kpn6x:/etc# uname -a
Linux my_machine 5.4.228-131.415.amzn2.x86_64 #1 SMP Tue Dec 20 12:51:02 UTC 2022 x86_64 GNU/Linux

We use the solutoion provided by VinceAvery to solve the issue. (that works)

This issue will impact us when we have such kind of code: message is Type

    public override void FromApp(Message message, SessionID sessionID)
    {
        if (message is MarketDataRequest marketDataRequest) // THIS LINE WILL FAIL, EVEN IT SHOULD BE RIGHT
        {
            HandleMarketDataRequest(marketDataRequest);
        }
        else if (message is BusinessMessageReject businessMessageReject) // THIS LINE WILL FAIL, EVEN IT SHOULD BE RIGHT
        {
            HandleBusinessMessageReject(businessMessageReject);
        }
    }

The reason for it, i think it's because the .NET team has made changes to the implementation of IndexOf method, dotnet/runtime#43736

and more docs from Microsoft, https://learn.microsoft.com/en-us/dotnet/core/extensions/globalization-icu

hope it helps.

[gbirchmeier]

I'm going to reopen this, until everyone agrees it has been fixed.

Current master is on .NET 6. We need to verify on Linux before the next release.

[Rob-Hague]

Some points

1. The line pointed out in the OP (int fieldvalend = msgstr.IndexOf((char)1, pos);) is already performing an ordinal comparison. This has been in master since September 2018 (Fix Message load error within a Linux-based Docker container #448), i.e. there is no fix to make there.
2. The .NET 5 breaking change was to move from NLS to ICU on Windows, so (in theory) there is no link between the .NET 5 change and problems experienced on Linux.
3. Problems may arise in string comparison/searching because SOH is a weightless character in unicode (see Strange Contains and IndexOf handling of "\0" in .NET 5.0 dotnet/runtime#46569 (comment)). Thus the library should always deal in ordinal operations on message strings. For that I have opened Use ordinal string operations #766.

[Falcz]

Hey all, thanks for taking a look into this! Any movement on #766? (looks like it was initially approved)

[caoyang1024]

Some points

1. The line pointed out in the OP (int fieldvalend = msgstr.IndexOf((char)1, pos);) is already performing an ordinal comparison. This has been in master since September 2018 (Fix Message load error within a Linux-based Docker container #448), i.e. there is no fix to make there.
2. The .NET 5 breaking change was to move from NLS to ICU on Windows, so (in theory) there is no link between the .NET 5 change and problems experienced on Linux.
3. Problems may arise in string comparison/searching because SOH is a weightless character in unicode (see Strange Contains and IndexOf handling of "\0" in .NET 5.0 dotnet/runtime#46569 (comment)). Thus the library should always deal in ordinal operations on message strings. For that I have opened Use ordinal string operations #766.

hi, regarding point 3, don't we need to change IndexOf() to use ordinal comparison as well?

[Rob-Hague]

Yes, #766 covers that. Do you see any places which have been missed?

[Falcz]

Looks good to me.

As of the change all IndexOf usages either rely on a char value (which is culture invariant under the hood) or specify Ordinal comparison.

QuickFIXn/Fields/Converters/DateTimeConverter.cs:            int i = str.IndexOf('.');
QuickFIXn/Fields/Converters/DateTimeConverter.cs:                int n = dec.Contains('+') ? dec.IndexOf('+') : dec.IndexOf('-');
QuickFIXn/Message/Message.cs:                int tagend = msgstr.IndexOf('=', pos);
QuickFIXn/Message/Message.cs:                int fieldvalend = msgstr.IndexOf((char)1, pos);
QuickFIXn/SessionFactory.cs:                        int offset = setting.Key.IndexOf('.');
QuickFIXn/Settings.cs:            return s.IndexOf('=') != -1;
QuickFIXn/Transport/StreamFactory.cs:            int index = data.IndexOf("200", StringComparison.Ordinal);

[gbirchmeier]

I think this one is all covered by #766 now, right? I'm closing this. If I am wrong, please let me know, or create a new issue and I will jump on it quickly.