Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String.IndexOf and String.LastIndexOf do not behave consistently across OS platforms #774

Closed
NightOwl888 opened this issue Dec 11, 2019 · 4 comments
Labels
area-System.Globalization untriaged New issue has not been triaged by the area owner

Comments

@NightOwl888
Copy link

Here are a couple of tests that have different behavior on Windows, macOS and Linux. The failure always seems to happen when StringComparison.CurrentCulture is used. The results on Azure DevOps are as follows:

Platform OS Result
.netcoreapp3.0 Windows Pass
.netcoreapp3.0 Linux Fail
.netcoreapp3.0 macOS Fail
.netcoreapp2.2 Windows Pass
.netcoreapp2.2 Linux Fail
.netcoreapp2.2 macOS Fail
.netcoreapp2.0 Windows Pass
.netcoreapp2.0 Linux Fail
.netcoreapp2.0 macOS Fail
.net451 Windows Pass
[Test]
public void Test_IndexOf_String_Int32_CultureSensitivity_()
{
    string fixture = "ዬ፡ዶጶቝአሄኢቌጕኬ\u124fቖኋዘዻ፡ሆገኅጬሷ\u135cቔቿ፺ዃጫቭዄ";
    string searchFor = "ሄኢቌጕኬ\u124fቖኋዘዻ";

    CultureInfo originalCulture = CultureInfo.CurrentCulture;
    try
    {
        CultureInfo.CurrentCulture = new CultureInfo("ru-MD");

        Assert.AreEqual(6, fixture.IndexOf(searchFor, 4, StringComparison.Ordinal));
        Assert.AreEqual(-1, fixture.IndexOf(searchFor, 4, StringComparison.CurrentCulture));

    }
    finally
    {
        CultureInfo.CurrentCulture = originalCulture;
    }
}

[Test]
public void Test_LastIndexOf_String_Int32_CultureSensitivity_()
{
    string fixture = "ዬ፡ዶጶቝአሄኢቌጕኬ\u124fቖኋዘዻ፡ሆገኅጬሷ\u135cቔቿ፺ዃጫቭዄ";
    string searchFor = "ሄኢቌጕኬ\u124fቖኋዘዻ";

    CultureInfo originalCulture = CultureInfo.CurrentCulture;
    try
    {
        CultureInfo.CurrentCulture = new CultureInfo("ru-MD");

        Assert.AreEqual(6, fixture.LastIndexOf(searchFor, 20, StringComparison.Ordinal));
        Assert.AreEqual(-1, fixture.LastIndexOf(searchFor, 20, StringComparison.CurrentCulture));

    }
    finally
    {
        CultureInfo.CurrentCulture = originalCulture;
    }
}

Do note these tests were intentionally made evil to try to find bugs in our own IndexOf implementations, they aren't intended as real-world data, but extreme edge cases. Whatever the case, I would expect StringComparison.CurrentCulture to behave consistently across operating systems.

I briefly scanned the documentation for IndexOf and the Best Practices for Using Strings in .NET, but don't see any obvious references to this difference in behavior across operating systems. Is this intentional, or a bug?

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels Dec 11, 2019
@Clockwork-Muse
Copy link
Contributor

Whatever the case, I would expect StringComparison.CurrentCulture to behave consistently across operating systems.

This is by design. Perhaps we should surface this more in the documentation?

As much as I think I may have preferred to have consistent behavior as well, you're not even guaranteed consistent behavior on Windows either (for a variety of reasons).

@NightOwl888
Copy link
Author

Great. Exactly what I needed to know. Inconsistent behavior does seem to make StringComparer.CurrentCulture practically useless for applications built on .NET Core/.NET Stanadard, though.

Yes, I suggest improving visibility of this behavior in the docs so it can be taken into account when designing tests.

@Clockwork-Muse
Copy link
Contributor

@NightOwl888 - CurrentCulture-type stuff is mostly meant for user facing things, though, which is a much larger can of worms in the first place.

@tarekgh
Copy link
Member

tarekgh commented Jun 16, 2020

As indicated in the discussions this is by design as .NET depends on the OS or ICU for globalization support. We are improving the story in .NET 5.0 as we started supporting ICU on Windows which will allow getting more consistent results when running on different OS's. Also, we are going to support App local ICU which can be used to ensure exact behavior when running anywhere.

@tarekgh tarekgh closed this as completed Jun 16, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Globalization untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

5 participants