Skip to content
This repository has been archived by the owner on Nov 29, 2018. It is now read-only.

Supporting custom cultures that are not pre-defined in options.SupportedCultures #237

Closed
chaim770 opened this issue Apr 13, 2016 · 16 comments

Comments

@chaim770
Copy link

Following this StackOverflow question:
http://stackoverflow.com/questions/36590969/enabling-client-based-culture-in-asp-net-core

It's currently impossible to let the CultureInfo be determined automatically from what the client specify in the Accept-Language header (as it used to be using <system.web.globalization enableClientBasedCulture> in Web.config).

The way it is now, I have to pre-register all the cultures I'm planning to support, which renders AcceptLanguageHeaderRequestCultureProvider quite useless.

The following is the culprit:

!supportedCultures.Any(supportedCulture => string.Equals(supportedCulture.Name, name, StringComparison.OrdinalIgnoreCase)))

@hishamco
Copy link
Contributor

I think after design change that discussed here #111 we should to register the supported culture instead of looking into Culture Info list which is removed after this change

@omuleanu
Copy link

@hishamco this change is already done, it's in rc1-final, the version that we're having trouble with

@hishamco
Copy link
Contributor

I know @omuleanu, but while web.config is no longer used, is there any reason to have such enableClientBasedCulture?!!

@omuleanu
Copy link

@hishamco yes, a lot of people need that, basically everyone that was/is using culture=auto in web.config and want the same behavior after porting their app to the new asp.net

I need it, to automatically get the user's ShortDatePattern, ShortTimePattern, DateTimeFormat, NumberDecimalSeparator, TimeSeparator

@hishamco
Copy link
Contributor

There 're a lot of changes in ASP.NET Core, and there's no guarantee that every feature before will be the same in the new world, nothing but what if you create a culture info list from Accept-Language header and pass it to RequestLocalizationOptions this way it collect the cultures automatically instead of hard coded.
I'm not sure if there's a change in the future to support this out of the box, @DamianEdwards any idea sir?

@Eilon
Copy link
Member

Eilon commented Apr 13, 2016

This is currently by design in ASP.NET Core for the reasons mentioned in #111 (comment).

If we do this, it would most likely be a different middleware, or perhaps an option on the current middleware, but it would have negative performance implications.

@chaim770
Copy link
Author

@Eilon

it would have negative performance implications.

Why? Can you please elaborate?

@Eilon
Copy link
Member

Eilon commented Apr 13, 2016

@chaim770 I go into more detail in #111 (comment). The key reason is that ASP.NET has to cache every culture, so it's a trade-off between not caching and having slow performance, or caching but having the cache grow infinitely. Neither is a good option, so we chose to cache, but to limit the size of the cache by requiring apps to specify which items can be cached.

@hishamco
Copy link
Contributor

@elion IMHO if the Accept-Language sent with tens of cultures, typically you’re only interested in the first one for setting the language as that’s the primary language. In this case we avoid the performance impact. If it's need I can write a simple middlware in my repo or in the entropy repo to prove the point

@DamianEdwards
Copy link
Member

That's not the point. The issue was a single client sending lots of requests (000's) for different cultures. Creating culture objects isn't cheap so we avoid doing it unless we need to. Culture works differently on .NET Core than it did in .NET Framework on Windows, where the list was fixed based on what the OS supported. That said, the logic in this middleware is pretty trivial, and if an app wants to have the format culture set to whatever the client asks for it's very easy for the app to add that custom logic in a simple middleware.

@hishamco
Copy link
Contributor

I agree with you that creating a culture object isn't cheap, but what I suggest is a workaround to migrate his old ASP.NET Application to ASP.NET Core 😄

@hikalkan
Copy link

hikalkan commented Dec 9, 2016

In our multitenant application, a tenant can add a new language from UI and edit localization texts for it. But the tenant can not switch to the new language unless we start IIS (we definetely don't want that). I think this is a very common case and there should be a built-in solution.
Thanks.

@hishamco
Copy link
Contributor

hishamco commented Mar 4, 2017

As I can see here https://github.com/aspnet/Localization/blob/dev/src/Microsoft.AspNetCore.Localization/AcceptLanguageHeaderRequestCultureProvider.cs#L42-L47, we are interest to the top MaximumAcceptLanguageHeaderValuesToTry even the client has tens of cultures, so why this has a performance impact?!!

@Eilon
Copy link
Member

Eilon commented Mar 5, 2017

@hishamco generally speaking, accepting unbounded inputs is a bad practice, whether for perf or security.

@hishamco
Copy link
Contributor

hishamco commented Mar 5, 2017

Either ways the AcceptLanguageHeaderRequestCultureProvider accepts the user cultures that set via Accept-Language header, whether they are correct or not, so if AcceptLanguageHeaderRequestCultureProvider is the winning provider, after that we set the current culture if its correct or fallback to the default culture.

What I mean in my last comment is we can consider the provided culture if it's correct while we care about the top 3 which is the default value

Regardless of what I wrote before while the aim of this issue is Supporting custom cultures that are not pre-defined in options.SupportedCultures, so we can go further by allowing the devs and applications authors to provide a custom cultures via configuration file, using Configuration & Options APIs, like what we have seen in EF, in this case we reduce (or eliminate) the impact because these cultures values will provided from the application itself not the end users themselves

@aspnet-hello
Copy link

This issue was moved to dotnet/aspnetcore#2649

@aspnet aspnet locked and limited conversation to collaborators Jan 2, 2018
@aspnet-hello aspnet-hello removed this from the Backlog milestone Jan 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants