turquoiseowl / i18n

Smart internationalization for ASP.NET
Other
556 stars 156 forks source link

ExtractLangTagFromUrl broke url if url not contains language. #289

Closed VitaliySmolyakov closed 7 years ago

VitaliySmolyakov commented 7 years ago

Hello.
To see this issue add to test LanguageTagTests.ExtractLangTagFromUrl following lines:

ExtractLangTagFromUrlHelper("/Api/City", "Api", "/City"); // Pass!
ExtractLangTagFromUrlHelper("/Api/City", "", "/Api/City"); // I expected. It failed.

So it extracted "Api" as langtag and url become "/City". I expect from this function to only extract tag for language that is in currently supported languages list LanguageHelpers.GetAppLanguages(). What do you think about it?

Thanks for your great work.

turquoiseowl commented 7 years ago

Yes, this is issue has been raised a few times before.

Say the URL is /fr-CH/Blog and you have an app language fr but not fr-CH? You clearly want fr-CH to be extracted even though there isn't an exact match to LanguageHelpers.GetAppLanguages().

The way to solve this would be run the language matching algorithm during ExtractLangTagFromUrl. However, because the code is performance sensitive (esp. during processing of outgoing requests when it is called for any/every URL found in the response) I found a valid reason for not doing the work to implement that:

        /// <remarks>
        /// This method does not check for the validity of the returned langtag other than
        /// it matching the pattern of a langtag as supported by this LanguageTag class.
        /// </remarks>

As this issue keeps cropping up, and IIRC each time it was for /api/..., a solution might be a hack excludes /api as the langtag?

turquoiseowl commented 7 years ago

Ref: #271 #240

VitaliySmolyakov commented 7 years ago

Thanks for your answer. If that is known problem then ok. I did not guessed to search "Api" in issues. Would be good to mention this problem in readme "URL Localization" section.

turquoiseowl commented 7 years ago

I've edited the default UrlLocalizer.QuickUrlExclusionFilter setting to now exclude '/api/...' urls by default from url localization.