Fix NLS ordinal casing of supplementary characters to agree with OrdinalIgnoreCase by tarekgh · Pull Request #134149 · dotnet/runtime

tarekgh · 2026-09-17T18:17:45Z

Summary

Under Windows NLS, ordinal casing and ordinal comparison are backed by two different Windows APIs that disagree for some supplementary scalars:

ToUpperOrdinal / ToLowerOrdinal map via LCMapStringEx, which casts the Deseret upper/lower pairs (for example U+10428 -> U+10400).
OrdinalIgnoreCase comparison uses CompareStringOrdinal, which treats those same supplementary pairs as not equal.

Because of this Windows-level incompatibility, the canonicalization contract was broken in NLS mode: ToUpperOrdinal(a) == ToUpperOrdinal(b) no longer matched string.Equals(a, b, StringComparison.OrdinalIgnoreCase). Two strings that ordinal-ignore-case comparison considered different could be folded to the same value by ordinal casing (and vice versa).

Fix

The change is NLS only and adds no casing tables (no binary size impact). After the normal casing pass, any supplementary pair that casing changed is validated against CompareStringOrdinal; if the cased value fell outside its OrdinalIgnoreCase class, the original scalar is restored. This reconciles ordinal casing output with the comparison oracle without carrying static data.

Applies to the string/Span ToUpperOrdinal/ToLowerOrdinal paths and to Rune.ToUpperOrdinal/Rune.ToLowerOrdinal.

ICU mode is not touched at all.

Performance (NLS mode only)

Release benchmarks, matched hosts differing only by System.Private.CoreLib, DOTNET_SYSTEM_GLOBALIZATION_USENLS=1:

Scenario	Ratio	Notes
ASCII	~1.0	Fast path unchanged (ratios shown are measurement noise)
BMP (no surrogates)	~1.09	One added vectorized surrogate scan
Emoji (surrogate-dense)	~1.24	Unchanged surrogate runs are bulk-skipped
Deseret (changed supplementary pairs)	higher	Rare correctness path; one native validation per genuinely changed pair

The added cost is confined to NLS mode. ASCII keeps its fast path, pure BMP adds a single vectorized scan, and unchanged surrogate runs are skipped in bulk so surrogate-dense text does not pay per-pair. The overhead is acceptable: this behavior ships as part of a new feature in .NET 11, the common ICU path is unaffected, and the alternative (embedding casing data) would increase binary size, which we want to avoid.

Tests

Added an NLS-specific test covering the full Deseret block U+10400..U+1044F that asserts String/Span/Rune ordinal casing agree with OrdinalIgnoreCase equality (canonicalization and collision), plus mixed-string and unpaired-high-surrogate cases. Focused OrdinalCasingTests: 91 passed, 0 failed.

Resolves #133950

…nalIgnoreCase Under Windows NLS, ordinal casing (ToUpperOrdinal/ToLowerOrdinal) uses LCMapStringEx while ordinal comparison uses CompareStringOrdinal. These two Windows APIs disagree for some supplementary scalars: LCMapStringEx maps the Deseret upper/lower pairs (for example U+10428 to U+10400), but CompareStringOrdinal with IgnoreCase treats those pairs as unequal. That broke the canonicalization contract where ToUpperOrdinal(a) equals ToUpperOrdinal(b) if and only if OrdinalIgnoreCase considers a and b equal. The fix is NLS only and adds no casing tables. After casing, changed supplementary pairs are validated against CompareStringOrdinal and restored to their original scalar when the mapping moved them outside their OrdinalIgnoreCase class. ASCII keeps its fast path, pure BMP input adds a single vectorized surrogate scan, and unchanged surrogate runs are skipped in bulk. ICU mode is untouched.

azure-pipelines · 2026-09-17T18:18:28Z

Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-09-17T18:18:55Z

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

Copilot

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes NLS ordinal casing so supplementary-character casing agrees with OrdinalIgnoreCase.

Changes:

Adds NLS validation/restoration for supplementary casing.
Updates Rune ordinal casing.
Adds comprehensive Deseret NLS tests.

File summaries

File	Description
src/libraries/System.Runtime/tests/System.Globalization.Tests/System/Globalization/OrdinalCasingTests.cs	Updated as part of this pull request.
src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs	Updated as part of this pull request.
src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs	Updated as part of this pull request.

Review details

Files reviewed: 3/3 changed files
Comments generated: 1
Review effort level: Lite

tarekgh · 2026-09-17T18:43:46Z

CC @javiercn

Copilot

🔵 Needs a closer look

The changes require final human review because they are too complex or risky for automated approval.

Review details

Suppressed comments (1)

src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs:1616

The preceding comment is now inaccurate for the NLS branch: CharUnicodeInfo.ToUpper is only the initial scalar mapping, and this new code may restore the original scalar because CompareStringOrdinal can disagree with it. Please update the comment to explain that NLS validates the mapping against the OrdinalIgnoreCase class.

            // Supplementary characters use the same simple scalar mapping as OrdinalIgnoreCase comparisons.
            uint upper = CharUnicodeInfo.ToUpper(value._value);
            return UnsafeCreate(GlobalizationMode.UseNls
                ? Ordinal.PreserveNlsOrdinalCasingClass(value._value, upper)

Files reviewed: 3/3 changed files
Comments generated: 0 new
Review effort level: Lite

…ing-supplementary

vcsjones

This looks acceptable. I did some additional tests using Adlam and Vithkuqi.

tarekgh · 2026-09-18T22:38:31Z

/backport to release/11.0

github-actions · 2026-09-18T22:38:41Z

Started backporting to release/11.0 (link to workflow run)

@tarekgh

…agree with OrdinalIgnoreCase (#134229) Backport of #134149 to release/11.0 /cc @tarekgh ## Customer Impact - [ ] Customer reported - [x] Found internally Ordinal casing is a new feature introduced in .NET 11. Customers using the new APIs on Windows with NLS globalization can receive incorrect results for supplementary-plane characters, including scripts such as Deseret, Adlam, and Vithkuqi. Two strings considered distinct by OrdinalIgnoreCase can be converted to the same ordinal-cased value, violating the documented canonicalization contract and potentially causing incorrect matching, deduplication, hashing, or key normalization. The issue is difficult for applications to work around and creates behavior inconsistent with ICU-based platforms. The fix is narrowly scoped to NLS mode, introduces no additional API changes, and aligns casing with the existing CompareStringOrdinal behavior. The issue found by aspnet. Look at the comment for more info dotnet/aspnetcore#68912 (comment). #133950 ## Regression - [ ] Yes - [x] No [If yes, specify when the regression was introduced. Provide the PR or commit if known.] ## Testing All regression tests are passed, additionally added more tests to cover the issue we are fixing ## Risk Low, this fixing issue in a new feature introduced in net11, additionally the change is scoped to NLS mode which is not the default mode (ICU mode is the default mode). Co-authored-by: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings September 17, 2026 18:17

github-actions Bot added the area-System.Globalization label Sep 17, 2026

Copilot started reviewing on behalf of tarekgh September 17, 2026 18:18 View session

dotnet-policy-service Bot assigned tarekgh Sep 17, 2026

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs Outdated

Allow oversized destination buffers and drop redundant comments

ef8e22a

Copilot AI review requested due to automatic review settings September 17, 2026 18:50

Copilot started reviewing on behalf of tarekgh September 17, 2026 18:50 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into fix-nls-ordinal-cas…

6dc4011

…ing-supplementary

build-analysis Bot mentioned this pull request Sep 18, 2026

System.Net.Quic.QuicException : Connection handshake was canceled due to the configured timeout #133645

Open

vcsjones reviewed Sep 18, 2026

View reviewed changes

Comment thread src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs

vcsjones approved these changes Sep 18, 2026

View reviewed changes

tarekgh merged commit f881b48 into dotnet:main Sep 18, 2026
135 of 138 checks passed

github-actions Bot mentioned this pull request Sep 18, 2026

[release/11.0] Fix NLS ordinal casing of supplementary characters to agree with OrdinalIgnoreCase #134229

Merged

2 of 4 tasks

dotnet-maestro Bot mentioned this pull request Sep 19, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#9527

Merged

dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 19, 2026

Navigation Menu

Fix NLS ordinal casing of supplementary characters to agree with OrdinalIgnoreCase - #134149

Fix NLS ordinal casing of supplementary characters to agree with OrdinalIgnoreCase#134149
tarekgh merged 3 commits into
dotnet:mainfrom
tarekgh:fix-nls-ordinal-casing-supplementary

tarekgh commented Sep 17, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Sep 17, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

tarekgh commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

vcsjones left a comment

Uh oh!

Uh oh!

tarekgh commented Sep 18, 2026

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tarekgh commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Performance (NLS mode only)

Tests

Uh oh!

azure-pipelines Bot commented Sep 17, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

tarekgh commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

Uh oh!

Uh oh!

vcsjones left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tarekgh commented Sep 18, 2026

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tarekgh commented Sep 17, 2026 •

edited

Loading