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

[release/6.0-staging] Fix Ordinal Ignore Case string compare #84386

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Apr 5, 2023

Backport of #71022 to release/6.0-staging

/cc @tarekgh

Customer Impact

A customer has reported a regression in .NET 6 where sorting with ordinal ignore case produces different results compared to .NET 5.0 and .NET Framework. The issue only arises when dealing with non-ASCII strings that contain characters with support casing. This can be challenging for the customer to troubleshoot and may manifest in various scenarios, such as deserialization across frameworks or user applications that rely on algorithms like binary trees or sorting.
Example of the broken behavior, when sorting Ѱa and ѰB, ѰB will come before Ѱa

The fix for this issue was already included in .NET 7.0. While we previously tried to backport the fix through a pull request PR, it was recommended that we wait and observe if more users experience the same problem. However, as we have received reports and requests from users to bring the fix to .NET 6.0, we are currently considering this option. More information can be found at #84251.

Testing

Have done manual testing to ensure we are getting the right behavior. I have added more tests to ensure I catch this problem if it happens again in the future. The change has passed all regression tests.

Risk

The risk associated with this change is low, as it is specific to the problematic scenario and is intended to correct the behavior. I do not anticipate any negative impact or interference with other processes. Additionally, since we have already implemented this fix successfully in .NET 7.0, the risk level is further reduced, given that there were no adverse effects reported following the implementation.

@ghost
Copy link

ghost commented Apr 5, 2023

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #71022 to release/6.0-staging

/cc @tarekgh

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Globalization

Milestone: -

@tarekgh tarekgh added the Servicing-consider Issue for next servicing release review label Apr 5, 2023
@tarekgh tarekgh added this to the 6.0.x milestone Apr 5, 2023
@tarekgh tarekgh self-assigned this Apr 5, 2023
@carlossanlop
Copy link
Member

@tarekgh - The new check-service-labels CI action should start passing after the Servicing-approved label is added, which is done after Tactics approves this PR.

@tarekgh
Copy link
Member

tarekgh commented Apr 6, 2023

This is approved by email.

@carlossanlop carlossanlop modified the milestones: 6.0.x, 6.0.17 Apr 6, 2023
@carlossanlop
Copy link
Member

carlossanlop commented Apr 6, 2023

Requirements for merging:

  • Approved by Tactics: added the Servicing-approved label and the check-service-labels CI action passing.
  • Signed-off by at least one area owner.
  • Confirm that the CI failures are unrelated to this change.
  • Confirm if these changes require OOB package authoring changes or not.

@tarekgh
Copy link
Member

tarekgh commented Apr 6, 2023

The change here is in corelib which doesn't need OOB authoring. Additionally, the CI failures are not related to the change and are known issues.

@tarekgh
Copy link
Member

tarekgh commented Apr 6, 2023

The change here is in corelib which doesn't need OOB authoring. additionally, the CI failures are not related to the change and are known issues.

@tarekgh tarekgh closed this Apr 6, 2023
@tarekgh tarekgh reopened this Apr 6, 2023
@tarekgh tarekgh merged commit d619619 into release/6.0-staging Apr 6, 2023
@carlossanlop carlossanlop deleted the backport/pr-71022-to-release/6.0-staging branch April 6, 2023 17:54
@ghost ghost locked as resolved and limited conversation to collaborators May 6, 2023
@rbhanda rbhanda modified the milestones: 6.0.17, 6.0.18 Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants