Skip to content

Commit

Permalink
Fix UI Culture bug (Azure#1901)
Browse files Browse the repository at this point in the history
  • Loading branch information
xingsy97 authored Jan 17, 2024
1 parent 3a0788b commit c31946d
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 20 deletions.
1 change: 1 addition & 0 deletions src/Microsoft.Azure.SignalR.Common/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public static class QueryParameter
public const string OriginalPath = "asrs.op";
public const string ConnectionRequestId = "asrs_request_id";
public const string RequestCulture = "asrs_lang";
public const string RequestUICulture = "asrs_ui_lang";
}

public static class CustomizedPingTimer
Expand Down
13 changes: 11 additions & 2 deletions src/Microsoft.Azure.SignalR/HubHost/NegotiateHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public async Task<NegotiationResponse> Process(HttpContext context)
var claims = BuildClaims(context);
var request = context.Request;
var cultureName = context.Features.Get<IRequestCultureFeature>()?.RequestCulture.Culture.Name;
var uiCultureName = context.Features.Get<IRequestCultureFeature>()?.RequestCulture.UICulture.Name;
var originalPath = GetOriginalPath(request.Path);
var provider = _endpointManager.GetEndpointProvider(_router.GetNegotiateEndpoint(context, _endpointManager.GetEndpoints(_hubName)));

Expand All @@ -88,7 +89,11 @@ public async Task<NegotiationResponse> Process(HttpContext context)
return null;
}

var queryString = GetQueryString(request.QueryString.HasValue ? request.QueryString.Value.Substring(1) : null, cultureName);
var queryString = GetQueryString(
request.QueryString.HasValue ? request.QueryString.Value.Substring(1) : null,
cultureName,
uiCultureName
);

return new NegotiationResponse
{
Expand All @@ -99,7 +104,7 @@ public async Task<NegotiationResponse> Process(HttpContext context)
};
}

private string GetQueryString(string originalQueryString, string cultureName)
private string GetQueryString(string originalQueryString, string cultureName, string uiCultureName)
{
var clientRequestId = _connectionRequestIdProvider.GetRequestId();
if (clientRequestId != null)
Expand All @@ -112,6 +117,10 @@ private string GetQueryString(string originalQueryString, string cultureName)
{
queryString += $"&{Constants.QueryParameter.RequestCulture}={cultureName}";
}
if (!string.IsNullOrEmpty(uiCultureName))
{
queryString += $"&{Constants.QueryParameter.RequestUICulture}={uiCultureName}";
}

return originalQueryString != null
? $"{originalQueryString}&{queryString}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,10 @@ private static void ProcessQuery(string queryString, out string originalPath)
{
SetCurrentThreadCulture(culture.FirstOrDefault());
}
if (query.TryGetValue(Constants.QueryParameter.RequestUICulture, out var uiCulture))
{
SetCurrentThreadUiCulture(uiCulture.FirstOrDefault());
}
if (query.TryGetValue(Constants.QueryParameter.OriginalPath, out var path))
{
originalPath = path.FirstOrDefault();
Expand All @@ -277,9 +281,22 @@ private static void SetCurrentThreadCulture(string cultureName)
{
try
{
var requestCulture = new RequestCulture(cultureName);
CultureInfo.CurrentCulture = requestCulture.Culture;
CultureInfo.CurrentUICulture = requestCulture.UICulture;
CultureInfo.CurrentCulture = new CultureInfo(cultureName);
}
catch (Exception)
{
// skip invalid culture, normal won't hit.
}
}
}

private static void SetCurrentThreadUiCulture(string uiCultureName)
{
if (!string.IsNullOrEmpty(uiCultureName))
{
try
{
CultureInfo.CurrentUICulture = new CultureInfo(uiCultureName);
}
catch (Exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
<DefineConstants>MULTIFRAMEWORK</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
<DefineConstants>OS_WINDOWS</DefineConstants>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\Microsoft.Azure.SignalR.Tests.Common\Microsoft.Azure.SignalR.Tests.Common.csproj" />
</ItemGroup>
Expand Down
4 changes: 3 additions & 1 deletion test/Microsoft.Azure.SignalR.Tests/NegotiateHandlerFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ public async Task TestNegotiateHandlerRespectClientRequestCulture()
QueryString = "?endpoint=chosen"
};
features.Set<IHttpRequestFeature>(requestFeature);
var customCulture = new RequestCulture("ar-SA");
var customCulture = new RequestCulture("ar-SA", "en-US");
features.Set<IRequestCultureFeature>(
new RequestCultureFeature(customCulture,
new AcceptLanguageHeaderRequestCultureProvider()));
Expand All @@ -470,7 +470,9 @@ public async Task TestNegotiateHandlerRespectClientRequestCulture()
var negotiateResponse = await handler.Process(httpContext);

var queryContainsCulture = negotiateResponse.Url.Contains($"{Constants.QueryParameter.RequestCulture}=ar-SA");
var queryContainsUICulture = negotiateResponse.Url.Contains($"{Constants.QueryParameter.RequestUICulture}=en-US");
Assert.True(queryContainsCulture);
Assert.True(queryContainsUICulture);
}

[Theory]
Expand Down
41 changes: 27 additions & 14 deletions test/Microsoft.Azure.SignalR.Tests/ServiceContextFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,26 +199,39 @@ public void ServiceConnectionContextRemoteIpTest(string xff, bool canBeParsed, s
}

[Theory]
[InlineData("&asrs_lang=ar-SA", true, "ar-SA")]
[InlineData("&asrs_lang=zh-CN", true, "zh-CN")]
[InlineData("", false, null)]
[InlineData("&arsa_lang=", false, null)]
[InlineData("&arsa_lang=123", false, null)] // invalid culture won't change default en-US
public void ServiceConnectionContextCultureTest(string cultureQuery, bool isValid, string result)
// For Linux and Mac, `CultureInfo` ctor doesn't treat invalid culture string as an invalid one. See https://github.com/dotnet/runtime/issues/11590
// If `CultureInfo` ctor treats the culture string as an invalid one, the default culture won't be changed and `parsedCulture` will be ignored.
// Culture / UICulture by default is same as OS, typically en-US. See https://learn.microsoft.com/en-us/dotnet/api/system.threading.thread.currentuiculture?view=net-8.0&redirectedfrom=MSDN#remarks
[InlineData("&asrs_lang=ar-SA", true, "ar-SA", false, null)]
[InlineData("&asrs_lang=zh-CN", true, "zh-CN", false, null)]
[InlineData("&asrs_ui_lang=ar-SA", false, null, true, "ar-SA")]
[InlineData("&asrs_ui_lang=zh-CN", false, null, true, "zh-CN")]
[InlineData("&asrs_lang=ar-SA&asrs_ui_lang=zh-CN", true, "ar-SA", true, "zh-CN")]
[InlineData("&asrs_lang=zh-CN&asrs_ui_lang=ar-SA", true, "zh-CN", true, "ar-SA")]
#if OS_WINDOWS
[InlineData("", false, null, false, null)]
[InlineData("&arsa_lang=", false, null, false, null)]
[InlineData("&arsa_lang=123", false, null, false, null)]
[InlineData("&arsa_ui_lang=", false, null, false, null)]
[InlineData("&arsa_ui_lang=123", false, null, false, null)]
[InlineData("&asrs_lang=ar-SA&asrs_ui_lang=", true, "ar-SA", false, null)]
[InlineData("&asrs_lang=ar-SA&asrs_ui_lang=123", true, "ar-SA", false, null)]
[InlineData("&asrs_lang=&asrs_ui_lang=ar-SA", false, null, true, "ar-SA")]
[InlineData("&asrs_lang=123&asrs_ui_lang=ar-SA", false, null, true, "ar-SA")]
#endif
public void ServiceConnectionContextCultureTest(string cultureQuery, bool isCultureValid, string parsedCulture, bool isUiCultureValid, string parsedUiCulture)
{
var queryString = $"?{cultureQuery}";
var originalCulture = CultureInfo.CurrentCulture.Name;
var originalUiCulture = CultureInfo.CurrentUICulture.Name;

_ = new ClientConnectionContext(new OpenConnectionMessage("1", new Claim[0], EmptyHeaders, queryString));

var expectedCulture = isCultureValid ? parsedCulture : originalCulture;
var expectedUiCulture = isUiCultureValid ? parsedUiCulture : originalUiCulture;

if (isValid)
{
Assert.Equal(result, CultureInfo.CurrentCulture.Name);
}
else
{
Assert.Equal(originalCulture, CultureInfo.CurrentCulture.Name);
}
Assert.Equal(expectedCulture, CultureInfo.CurrentCulture.Name);
Assert.Equal(expectedUiCulture, CultureInfo.CurrentUICulture.Name);
}
}
}

0 comments on commit c31946d

Please sign in to comment.