Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

use realistic values for accept-language header #509

Merged
merged 3 commits into from

3 participants

@nemecec

Fix for "Accept-Language" header value parsing (when country is included). Currently Play assumes that "Accept-Language" header values have the same format as Java locale (e.g. "en_GB") while the real format uses a dash instead of an underscore (e.g. "en-GB").

@mbknor
Collaborator

If it is correct to do the replace, your fix does not do it correct. desiredLocales is used multiple times and you only replace the values the first time

@nemecec

True. Fixed now. Also, added a test for the partial match logic.

@nemecec

Found another bug, Lang.getLocale(String) method was assuming that the passed in String is language-only (e.g. "en"), but in reality it can be full locale String (e.g. "en_GB")

@pepite
Owner

pull request looks good to me

@mbknor
Collaborator

agree, now it looks much better

@pepite pepite merged commit 21052ae into playframework:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
40 framework/src/play/i18n/Lang.java
@@ -1,7 +1,10 @@
package play.i18n;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.Locale;
+
import play.Logger;
import play.Play;
import play.mvc.Http;
@@ -89,11 +92,16 @@ public static void change(String locale) {
* @param desiredLocales a collection of desired locales. If the collection is ordered, earlier locales are preferred over later ones.
* Locales should be of the form "[language]_[country" or "[language]", e.g. "en_CA" or "en".
* The locale strings are case insensitive (e.g. "EN_CA" is considered the same as "en_ca").
+ * Locales can also be of the form "[language]-[country", e.g. "en-CA" or "en".
+ * They are still case insensitive, though (e.g. "EN-CA" is considered the same as "en-ca").
* @return the closest matching locale. If no closest match for a language/country is found, null is returned
*/
- private static String findClosestMatch(Iterable<String> desiredLocales) {
+ private static String findClosestMatch(Collection<String> desiredLocales) {
+ ArrayList<String> cleanLocales = new ArrayList<String>(desiredLocales.size());
//look for an exact match
for (String a: desiredLocales) {
+ a = a.replace("-", "_");
+ cleanLocales.add(a);
for (String locale: Play.langs) {
if (locale.equalsIgnoreCase(a)) {
return locale;
@@ -101,14 +109,16 @@ private static String findClosestMatch(Iterable<String> desiredLocales) {
}
}
// Exact match not found, try language-only match.
- for (String a: desiredLocales) {
- if (a.indexOf("_") > 0) {
- a = a.substring(0, a.indexOf("_"));
+ for (String a: cleanLocales) {
+ int splitPos = a.indexOf("_");
+ if (splitPos > 0) {
+ a = a.substring(0, splitPos);
}
for (String locale: Play.langs) {
String langOnlyLocale;
- if (locale.indexOf("_") > 0) {
- langOnlyLocale = locale.substring(0, locale.indexOf("_"));
+ int localeSplitPos = locale.indexOf("_");
+ if (localeSplitPos > 0) {
+ langOnlyLocale = locale.substring(0, localeSplitPos);
} else {
langOnlyLocale = locale;
}
@@ -171,21 +181,29 @@ public static void setDefaultLocale() {
* associated to the current Lang.
*/
public static Locale getLocale() {
- String lang = get();
- Locale locale = getLocale(lang);
+ String localeStr = get();
+ Locale locale = getLocale(localeStr);
if (locale != null) {
return locale;
}
return Locale.getDefault();
}
- public static Locale getLocale(String lang) {
+ public static Locale getLocale(String localeStr) {
+ Locale langMatch = null;
for (Locale locale : Locale.getAvailableLocales()) {
- if (locale.getLanguage().equals(lang)) {
+ String lang = localeStr;
+ int splitPos = lang.indexOf("_");
+ if (splitPos > 0) {
+ lang = lang.substring(0, splitPos);
+ }
+ if (locale.toString().equalsIgnoreCase(localeStr)) {
return locale;
+ } else if (locale.getLanguage().equalsIgnoreCase(lang)) {
+ langMatch = locale;
}
}
- return null;
+ return langMatch;
}
}
View
52 framework/test-src/play/i18n/LangTest.java
@@ -1,16 +1,17 @@
package play.i18n;
+import static org.fest.assertions.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.Locale;
+
import org.junit.Test;
+
import play.Play;
import play.PlayBuilder;
import play.mvc.Http;
import play.test.FunctionalTest;
-import java.util.ArrayList;
-import java.util.Arrays;
-
-import static org.fest.assertions.Assertions.*;
-
public class LangTest {
@Test
@@ -48,7 +49,7 @@ public void testChange() {
@Test
public void testGet() {
new PlayBuilder().build();
- Play.langs = Arrays.asList("no", "en", "en_uk", "fr");
+ Play.langs = Arrays.asList("no", "en", "en_GB", "fr");
Lang.current.set(null);
Http.Response.current.set( new Http.Response());
@@ -61,44 +62,50 @@ public void testGet() {
Http.Request req = FunctionalTest.newRequest();
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("no");
+ assertLocale(new Locale("no"));
// check only with accept-language, without cookie value
req = FunctionalTest.newRequest();
req.headers.put("accept-language", new Http.Header("accept-language", "x"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("no");
+ assertLocale(new Locale("no"));
req = FunctionalTest.newRequest();
req.headers.put("accept-language", new Http.Header("accept-language", "no"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("no");
+ assertLocale(new Locale("no"));
req = FunctionalTest.newRequest();
req.headers.put("accept-language", new Http.Header("accept-language", "en"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en");
+ assertLocale(new Locale("en"));
req = FunctionalTest.newRequest();
req.headers.put("accept-language", new Http.Header("accept-language", "x,en"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en");
+ assertLocale(new Locale("en"));
+
+ req = FunctionalTest.newRequest();
+ req.headers.put("accept-language", new Http.Header("accept-language", "en-GB"));
+ Http.Request.current.set(req);
+ Lang.current.set(null);
+ assertLocale(new Locale("en", "GB"));
req = FunctionalTest.newRequest();
- req.headers.put("accept-language", new Http.Header("accept-language", "en_uk"));
+ req.headers.put("accept-language", new Http.Header("accept-language", "x,en-GB"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en_uk");
+ assertLocale(new Locale("en", "GB"));
req = FunctionalTest.newRequest();
- req.headers.put("accept-language", new Http.Header("accept-language", "x,en_uk"));
+ req.headers.put("accept-language", new Http.Header("accept-language", "x,en-US"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en_uk");
+ assertLocale(new Locale("en"));
// check with cookie value
@@ -111,7 +118,7 @@ public void testGet() {
req.headers.put("accept-language", new Http.Header("accept-language", "en"));
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en");
+ assertLocale(new Locale("en"));
cookie = new Http.Cookie();
cookie.name = "PLAY_LANG";
@@ -119,7 +126,7 @@ public void testGet() {
req.cookies.put(cookie.name, cookie);
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en");
+ assertLocale(new Locale("en"));
cookie = new Http.Cookie();
cookie.name = "PLAY_LANG";
@@ -127,16 +134,21 @@ public void testGet() {
req.cookies.put(cookie.name, cookie);
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en");
+ assertLocale(new Locale("en"));
cookie = new Http.Cookie();
cookie.name = "PLAY_LANG";
- cookie.value = "en_uk";
+ cookie.value = "en_GB";
req.cookies.put(cookie.name, cookie);
Http.Request.current.set(req);
Lang.current.set(null);
- assertThat(Lang.get()).isEqualTo("en_uk");
+ assertLocale(new Locale("en", "GB"));
}
+
+ private void assertLocale(Locale locale) {
+ assertThat(Lang.get()).isEqualTo(locale.toString());
+ assertThat(Lang.getLocale()).isEqualTo(locale);
+ }
}
Something went wrong with that request. Please try again.