Skip to content

Reject "root" culture name in ICU globalization mode - #133275

Open
tarekgh wants to merge 1 commit into
dotnet:mainfrom
tarekgh:reject-root-culture-132429
Open

Reject "root" culture name in ICU globalization mode#133275
tarekgh wants to merge 1 commit into
dotnet:mainfrom
tarekgh:reject-root-culture-132429

Conversation

@tarekgh

Copy link
Copy Markdown
Member

Summary

Fixes#132429.

CultureInfo.GetCultureInfo("root") was accepted in ICU globalization mode but produced an incomplete culture. ICU normalizes the CLDR root locale to an empty name, so the resulting CultureData had Name == "" without being the Invariant singleton. This caused two problems:

  1. Reading NumberFormat on it threw NullReferenceException from NumberFormatInfo.InitializeInvariantAndNegativeSignFlags because the negative sign was never populated.
  2. Resolving "root" before the invariant culture poisoned the "" cache slot, so later lookups of the invariant culture returned the broken instance.

Fix

The empty-string and "und" names are already short-circuited to the invariant culture in CultureData.GetCultureData, so an empty name reaching InitIcuCultureDataCore can only come from a name such as "root" that is not a valid culture. The change rejects that case, so GetCultureInfo("root") now throws CultureNotFoundException, matching the existing NLS behavior (Windows does not recognize root).

This is a single length check on the culture creation cold path (cache miss only), so there is no impact on the hot path or on any valid culture.

Behavior by globalization mode

ModeBeforeAfter
ICU (default)Incomplete culture, NRE on NumberFormat, cache poisoningCultureNotFoundException
NLSCultureNotFoundExceptionUnchanged
Invariant (predefined-only)CultureNotFoundExceptionUnchanged
Invariant, predefined-only disabledFabricated culture named root from invariant dataUnchanged

Tests

Added to System.Globalization.Tests.GetCultureInfoTests (gated on IsNotInvariantGlobalization):

  • GetCultureInfo_RootCultureName_Throws for root, ROOT, Root across all overloads and the CultureInfo constructor.
  • GetCultureInfo_RootCultureName_DoesNotPoisonInvariantCache using RemoteExecutor to verify that resolving root first leaves the invariant culture intact and that und still maps to invariant.

Verified locally on Windows x64 (ICU): the full GetCultureInfoTests class passes (65/65) with the fix, and the new tests fail without it.

CultureInfo.GetCultureInfo("root") was accepted in ICU mode but produced an
incomplete culture. ICU normalizes the CLDR "root" locale to an empty name, so
the resulting CultureData had Name == "" without being the Invariant singleton.
That caused a NullReferenceException when reading NumberFormat and poisoned the
invariant culture's "" cache slot when "root" was resolved before the invariant
culture.
The empty-string and "und" names are already short-circuited to the invariant
culture in GetCultureData, so an empty name in InitIcuCultureDataCore can only
come from a name such as "root" that is not a valid culture. Reject it so
GetCultureInfo("root") throws CultureNotFoundException, matching the existing
NLS behavior.
Fixesdotnet#132429
CopilotAI lite review requested due to automatic review settings September 4, 2026 22:46
@azure-pipelines

Copy link
Copy Markdown
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

Copy link
Copy Markdown
Contributor

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a narrowly-scoped correctness fix on a cold path with targeted regression tests validating both the exception behavior and the cache-poisoning scenario.

Pull request overview

This PR fixes an ICU-mode globalization edge case where CultureInfo.GetCultureInfo("root") could succeed but produce a broken culture (with Name == ""), leading to internal NullReferenceExceptions and poisoning the CultureInfo name cache entry for the invariant culture.

Changes:

  • Reject ICU-normalized empty locale names during ICU culture initialization to ensure "root" fails culture creation rather than producing an incomplete culture.
  • Add regression coverage ensuring "root" throws CultureNotFoundException across key entry points and does not poison the invariant culture cache (including verifying "und" still maps to invariant).
  • Gate the tests to only run when a non-invariant globalization backend is in use and RemoteExecutor is available.
File summaries
FileDescription
src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Icu.csAdds a guard to fail ICU culture initialization if ICU returns an empty locale name (preventing "root" from creating a broken culture and avoiding cache poisoning).
src/libraries/System.Runtime/tests/System.Globalization.Tests/CultureInfo/GetCultureInfo.csAdds regression tests verifying "root" throws and does not poison the invariant culture cache (and that "und" still maps to invariant).
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CultureInfo.GetCultureInfo("root") is an accepted but incomplete culture which can raise NullReferenceException on internals

2 participants

@tarekgh