Skip to content

Update RegionInfo data and Fix RegionInfo.CurrentRegion on Windows #33834

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.

Sign up for GitHub

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

Merged
tarekgh merged 4 commits into from
Mar 21, 2020
Merged

Update RegionInfo data and Fix RegionInfo.CurrentRegion on Windows #33834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
19d3eaa
Update RegionInfo data and Fix RegionInfo.CurrentRegion on Windows
tarekgh Mar 19, 2020
e77c9c3
Address the feedback
tarekgh Mar 20, 2020
731f74f
feedback
tarekgh Mar 20, 2020
30424e5
More Feedback addressing
tarekgh Mar 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/coreclr/src/System.Private.CoreLib/PinvokeAnalyzerExceptionList.analyzerdata
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ normaliz.dll!NormalizeString
<!-- On Nano these are stubs that return failure -->
user32.dll!GetProcessWindowStation
user32.dll!GetUserObjectInformationW

<!-- GetGeoInfo is supported by the analyzer complain for nit using GetGeoInfoW instead and we need to keep the style of not using 'W' in the names -->
kernel32.dll!GetGeoInfo
10 changes: 10 additions & 0 deletions src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Globalization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ internal static unsafe partial class Kernel32

internal const uint TIME_NOSECONDS = 0x00000002;

internal const int GEOCLASS_NATION = 16;
internal const int GEO_ISO2 = 4;
internal const int GEOID_NOT_AVAILABLE = -1;

internal const string LOCALE_NAME_USER_DEFAULT = null;
internal const string LOCALE_NAME_SYSTEM_DEFAULT = "!x-sys-default-locale";

Expand Down Expand Up @@ -133,6 +137,12 @@ internal static extern bool IsNLSDefinedString(
[DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
internal static extern int GetCalendarInfoEx(string? lpLocaleName, uint Calendar, IntPtr lpReserved, uint CalType, IntPtr lpCalData, int cchData, IntPtr lpValue);

[DllImport("kernel32.dll")]
internal static extern int GetUserGeoID(int geoClass);

[DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
internal static extern int GetGeoInfo(int location, int geoType, char* lpGeoData, int cchData, int LangId);

[DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
internal static extern bool EnumCalendarInfoExEx(EnumCalendarInfoProcExEx pCalInfoEnumProcExEx, string lpLocaleName, uint Calendar, string? lpReserved, uint CalType, void* lParam);

Expand Down
3 changes: 2 additions & 1 deletion src/libraries/Common/tests/CoreFx.Private.TestUtilities/System/PlatformDetection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public static partial class PlatformDetection
public static bool IsArgIteratorSupported => IsMonoRuntime || (IsWindows && IsNotArmProcess);
public static bool IsArgIteratorNotSupported => !IsArgIteratorSupported;
public static bool Is32BitProcess => IntPtr.Size == 4;
public static bool IsNotWindows => !IsWindows;

// Please make sure that you have the libgdiplus dependency installed.
// For details, see https://docs.microsoft.com/dotnet/core/install/dependencies?pivots=os-macos&tabs=netcore31#libgdiplus
Expand Down Expand Up @@ -206,7 +207,7 @@ private static bool GetSsl3Support()
private static bool GetIsRunningOnMonoInterpreter()
{
// This is a temporary solution because mono does not support interpreter detection
// within the runtime.
// within the runtime.
var val = Environment.GetEnvironmentVariable("MONO_ENV_OPTIONS");
return (val != null && val.Contains("--interpreter"));
}
Expand Down
12 changes: 8 additions & 4 deletions src/libraries/System.Globalization/tests/CultureInfo/CultureInfoAll.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Runtime.InteropServices;
using Microsoft.DotNet.RemoteExecutor;
using System.Text;
using Xunit;

Expand Down Expand Up @@ -631,10 +632,13 @@ public void GetCulturesTest(string cultureName, int lcid, string specificCulture
[Fact]
public void ClearCachedDataTest()
{
CultureInfo ci = CultureInfo.GetCultureInfo("ja-JP");
Assert.True((object) ci == (object) CultureInfo.GetCultureInfo("ja-JP"), "Expected getting same object reference");
ci.ClearCachedData();
Assert.False((object) ci == (object) CultureInfo.GetCultureInfo("ja-JP"), "expected to get a new object reference");
RemoteExecutor.Invoke(() =>
{
CultureInfo ci = CultureInfo.GetCultureInfo("ja-JP");
Assert.True((object) ci == (object) CultureInfo.GetCultureInfo("ja-JP"), "Expected getting same object reference");
ci.ClearCachedData();
Assert.False((object) ci == (object) CultureInfo.GetCultureInfo("ja-JP"), "expected to get a new object reference");
}).Dispose();
}

[Fact]
Expand Down
17 changes: 16 additions & 1 deletion src/libraries/System.Globalization/tests/System/Globalization/RegionInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void Ctor_InvalidName_ThrowsArgumentException(string name)
AssertExtensions.Throws<ArgumentException>("name", () => new RegionInfo(name));
}

[Fact]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows))]
public void CurrentRegion()
{
using (new ThreadCultureChange("en-US"))
Expand All @@ -63,6 +63,21 @@ public void CurrentRegion()
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))]
public void TestCurrentRegion()
{
RemoteExecutor.Invoke(() =>
{
RegionInfo ri = RegionInfo.CurrentRegion;
CultureInfo.CurrentCulture.ClearCachedData(); // clear the current region cached data

CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo("ja-JP");

// Changing the current culture shouldn't affect the default current region as we get it from Windows settings.
Assert.Equal(ri.TwoLetterISORegionName, RegionInfo.CurrentRegion.TwoLetterISORegionName);
}).Dispose();
}

[Theory]
[InlineData("en-US", "United States")]
[OuterLoop("May fail on machines with multiple language packs installed")] // see https://github.com/dotnet/runtime/issues/30132
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,5 +418,7 @@ private static string GetConsoleFallbackName(string cultureName)
internal bool IsWin32Installed => false;

internal bool IsReplacementCulture => false;

internal static CultureData GetCurrentRegionData() => CultureInfo.CurrentCulture._cultureData;
}
}
29 changes: 29 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Text;
using Internal.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace System.Globalization
{
Expand Down Expand Up @@ -714,5 +715,33 @@ internal bool IsReplacementCulture
return false;
}
}

internal static unsafe CultureData GetCurrentRegionData()
{
Span<char> geoIso2Letters = stackalloc char[10];

int geoId = Interop.Kernel32.GetUserGeoID(Interop.Kernel32.GEOCLASS_NATION);
if (geoId != Interop.Kernel32.GEOID_NOT_AVAILABLE)
{
int geoIsoIdLength;
fixed (char* pGeoIsoId = geoIso2Letters)
{
geoIsoIdLength = Interop.Kernel32.GetGeoInfo(geoId, Interop.Kernel32.GEO_ISO2, pGeoIsoId, geoIso2Letters.Length, 0);
}

if (geoIsoIdLength != 0)
{
geoIsoIdLength -= geoIso2Letters[geoIsoIdLength - 1] == 0 ? 1 : 0; // handle null termination and exclude it.

@danmoseley danmoseley Mar 20, 2020

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.

So sometimes it's null terminated and sometimes not? Why does it not return ERROR_INSUFFICIENT_BUFFER when it has no room to null terminate?

@tarekgh tarekgh Mar 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The doc is not ensuring returning null that is why I have to check to know if we got the null or not.
We are requesting the 2-letters country code and I am providing a buffer of length 10 which is far enough to ensure not running into the ERROR_INSUFFICIENT_BUFFER case even with null.

@jkotas jkotas Mar 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From what I can tell, this is always returning zero terminated string.

Would it be best to create the string from the pointer and not bother with using the returned size? I would be fine with not using the Span in that case because of there is no buffer math that can go wrong.

char* pGeoIso2Letters = stackalloc char[10];

if (Interop.Kernel32.GetGeoInfo(..., pGeoIso2Letters, ...) != 0)
{
    CultureData? cd = GetCultureDataForRegion(new string(pGeoIso2Letters), ...

@tarekgh tarekgh Mar 20, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I would be more conservative checking null as the doc didn't ensure that and I am not sure what happen if we run on a different version of Windows.

CultureData? cd = GetCultureDataForRegion(geoIso2Letters.Slice(0, geoIsoIdLength).ToString(), true);
if (cd != null)
{
return cd;
}
}
}

// Fallback to current locale data.
return CultureInfo.CurrentCulture._cultureData;
}
}
}
Loading