Skip to content
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.

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

ExtractLangTagFromUrl broke url if url not contains language. #289

Closed
VitaliySmolyakov opened this issue Aug 29, 2016 · 4 comments
Closed

Comments

@VitaliySmolyakov
Copy link

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

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

Ref: #271 #240

@VitaliySmolyakov
Copy link
Author

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants