Ensure language redirect conditions do not match for too low quality accepted languages #2045
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2045 +/- ##
==========================================
Coverage 97.34% 97.34%
Complexity 1363 1363
==========================================
Files 251 251
Lines 4825 4825
==========================================
Hits 4697 4697
Misses 128 128 ☔ View full report in Codecov by Sentry. |
…accepted languages
8262df1 to
be8cf56
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2043
In the initial implementation for language redirect conditions, I decided to let it match for any language present in the
Accept-Languageheader, with the only exception of*.Unfortunately, due to how browsers decide which languages to send in the
Accept-Languageheader, this resulted in unexpected matching rules when multiple rules are configured for different languages.For example, if you configure a rule for English and then a rule for Spanish, with an accept language like
es-ES,es;q=0.9,en-US;q=0.8,en;q=0.7,ja;q=0.6the English condition would positively match if it has more priority than the Spanish one, when intuition says that this header should match the Spanish condition.This PR changes the logic when matching languages, so that a minimum 0.9 quality is required for languages in the
Accept-Languageheader.In the example above, that would mean that only
es-ESandeswould end up being evaluated, and the rest would be ignored.As a side effect, that would mean that someone with the header
fr-FR,fr;q=0.9,en-US;q=0.8,en;q=0.7,ja;q=0.6would not match the English rule anymore, while it would with previous approach. However, the expected behavior would probably be that no rule matches, and instead redirection falls back to the default long URL, so this is probably correct.