AcceptHeaderLocaleContextResolver leniently handles invalid header value

Also falls back to language-only match among its supported locales now.

Issue: SPR-16500
Issue: SPR-16457

spring-web/src/main/java/org/springframework/http/HttpHeaders.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -473,6 +473,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
 	 * a list of supported locales you can pass the returned list to
 	 * {@link Locale#filter(List, Collection)}.
 	 * @since 5.0
+	 * @throws IllegalArgumentException if the value cannot be converted to a language range
 	 */
 	public List<Locale.LanguageRange> getAcceptLanguage() {
 		String value = getFirst(ACCEPT_LANGUAGE);
@@ -494,6 +495,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
 	 * {@link java.util.Locale.LanguageRange} to a {@link Locale}.
 	 * @return the locales or an empty list
 	 * @since 5.0
+	 * @throws IllegalArgumentException if the value cannot be converted to a locale
 	 */
 	public List<Locale> getAcceptLanguageAsLocales() {
 		List<Locale.LanguageRange> ranges = getAcceptLanguage();
@@ -879,7 +881,7 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
 	 * by the {@code Date} header.
 	 * <p>The date is returned as the number of milliseconds since
 	 * January 1, 1970 GMT. Returns -1 when the date is unknown.
-	 * @throws IllegalArgumentException if the value can't be converted to a date
+	 * @throws IllegalArgumentException if the value cannot be converted to a date
 	 */
 	public long getDate() {
 		return getFirstDate(DATE);

spring-web/src/main/java/org/springframework/web/server/i18n/AcceptHeaderLocaleContextResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,8 +23,9 @@ import java.util.Locale;
 import org.springframework.context.i18n.LocaleContext;
 import org.springframework.context.i18n.SimpleLocaleContext;
 import org.springframework.http.HttpHeaders;
-import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.lang.Nullable;
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.StringUtils;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
@@ -32,10 +33,11 @@ import org.springframework.web.server.ServerWebExchange;
  * specified in the "Accept-Language" header of the HTTP request (that is,
  * the locale sent by the client browser, normally that of the client's OS).
  *
- * <p>Note: Does not support {@code setLocale}, since the accept header
+ * <p>Note: Does not support {@link #setLocaleContext}, since the accept header
  * can only be changed through changing the client's locale settings.
  *
  * @author Sebastien Deleuze
+ * @author Juergen Hoeller
  * @since 5.0
  */
 public class AcceptHeaderLocaleContextResolver implements LocaleContextResolver {
@@ -51,11 +53,9 @@ public class AcceptHeaderLocaleContextResolver implements LocaleContextResolver
 	 * determined via {@link HttpHeaders#getAcceptLanguageAsLocales()}.
 	 * @param locales the supported locales
 	 */
-	public void setSupportedLocales(@Nullable List<Locale> locales) {
+	public void setSupportedLocales(List<Locale> locales) {
 		this.supportedLocales.clear();
-		if (locales != null) {
+		this.supportedLocales.addAll(locales);
-			this.supportedLocales.addAll(locales);
-		}
 	}
 
 	/**
@@ -82,42 +82,50 @@ public class AcceptHeaderLocaleContextResolver implements LocaleContextResolver
 		return this.defaultLocale;
 	}
 
+
 	@Override
 	public LocaleContext resolveLocaleContext(ServerWebExchange exchange) {
-		ServerHttpRequest request = exchange.getRequest();
+		List<Locale> requestLocales = null;
-		List<Locale> acceptableLocales = request.getHeaders().getAcceptLanguageAsLocales();
+		try {
-		if (this.defaultLocale != null && acceptableLocales.isEmpty()) {
+			requestLocales = exchange.getRequest().getHeaders().getAcceptLanguageAsLocales();
-			return new SimpleLocaleContext(this.defaultLocale);
 		}
-		Locale requestLocale = acceptableLocales.isEmpty() ? null : acceptableLocales.get(0);
+		catch (IllegalArgumentException ex) {
-		if (isSupportedLocale(requestLocale)) {
+			// Invalid Accept-Language header: treat as empty for matching purposes
-			return new SimpleLocaleContext(requestLocale);
 		}
-		Locale supportedLocale = findSupportedLocale(request);
+		return new SimpleLocaleContext(resolveSupportedLocale(requestLocales));
-		if (supportedLocale != null) {
-			return new SimpleLocaleContext(supportedLocale);
-		}
-		return (this.defaultLocale != null ? new SimpleLocaleContext(this.defaultLocale) :
-				new SimpleLocaleContext(requestLocale));
-	}
-
-	private boolean isSupportedLocale(@Nullable Locale locale) {
-		if (locale == null) {
-			return false;
-		}
-		List<Locale> supportedLocales = getSupportedLocales();
-		return (supportedLocales.isEmpty() || supportedLocales.contains(locale));
 	}
 
 	@Nullable
-	private Locale findSupportedLocale(ServerHttpRequest request) {
+	private Locale resolveSupportedLocale(@Nullable List<Locale> requestLocales) {
-		List<Locale> requestLocales = request.getHeaders().getAcceptLanguageAsLocales();
+		if (CollectionUtils.isEmpty(requestLocales)) {
+			return this.defaultLocale;  // may be null
+		}
+		List<Locale> supported = getSupportedLocales();
+		if (supported.isEmpty()) {
+			return requestLocales.get(0);  // never null
+		}
+
+		Locale languageMatch = null;
 		for (Locale locale : requestLocales) {
-			if (getSupportedLocales().contains(locale)) {
+			if (supported.contains(locale)) {
+				// Full match: typically language + country
 				return locale;
 			}
+			else if (languageMatch == null) {
+				// Let's try to find a language-only match as a fallback
+				for (Locale candidate : supported) {
+					if (!StringUtils.hasLength(candidate.getCountry()) &&
+							candidate.getLanguage().equals(locale.getLanguage())) {
+						languageMatch = candidate;
+					}
+				}
+			}
 		}
-		return null;
+		if (languageMatch != null) {
+			return languageMatch;
+		}
+
+		return (this.defaultLocale != null ? this.defaultLocale : requestLocales.get(0));
 	}
 
 	@Override

spring-web/src/main/java/org/springframework/web/server/i18n/FixedLocaleContextResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,12 +26,11 @@ import org.springframework.util.Assert;
 import org.springframework.web.server.ServerWebExchange;
 
 /**
- * {@link LocaleContextResolver} implementation that always returns
+ * {@link LocaleContextResolver} implementation that always returns a fixed locale
- * a fixed default locale and optionally time zone.
+ * and optionally time zone. Default is the current JVM's default locale.
- * Default is the current JVM's default locale.
  *
- * <p>Note: Does not support {@code setLocale(Context)}, as the fixed
+ * <p>Note: Does not support {@link #setLocaleContext}, as the fixed locale and
- * locale and time zone cannot be changed.
+ * time zone cannot be changed.
  *
  * @author Sebastien Deleuze
  * @since 5.0
@@ -71,6 +70,7 @@ public class FixedLocaleContextResolver implements LocaleContextResolver {
 		this.timeZone = timeZone;
 	}
 
+
 	@Override
 	public LocaleContext resolveLocaleContext(ServerWebExchange exchange) {
 		return new TimeZoneAwareLocaleContext() {

spring-web/src/test/java/org/springframework/web/server/i18n/AcceptHeaderLocaleContextResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,37 +22,39 @@ import java.util.Locale;
 
 import org.junit.Test;
 
+import org.springframework.http.HttpHeaders;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.web.server.ServerWebExchange;
 
 import static java.util.Locale.*;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 
 /**
  * Unit tests for {@link AcceptHeaderLocaleContextResolver}.
  *
  * @author Sebastien Deleuze
+ * @author Juergen Hoeller
  */
 public class AcceptHeaderLocaleContextResolverTests {
 
-	private AcceptHeaderLocaleContextResolver resolver = new AcceptHeaderLocaleContextResolver();
+	private final AcceptHeaderLocaleContextResolver resolver = new AcceptHeaderLocaleContextResolver();
 
 
 	@Test
-	public void resolve() throws Exception {
+	public void resolve() {
 		assertEquals(CANADA, this.resolver.resolveLocaleContext(exchange(CANADA)).getLocale());
 		assertEquals(US, this.resolver.resolveLocaleContext(exchange(US, CANADA)).getLocale());
 	}
 
 	@Test
-	public void resolvePreferredSupported() throws Exception {
+	public void resolvePreferredSupported() {
 		this.resolver.setSupportedLocales(Collections.singletonList(CANADA));
 		assertEquals(CANADA, this.resolver.resolveLocaleContext(exchange(US, CANADA)).getLocale());
 	}
 
 	@Test
-	public void resolvePreferredNotSupported() throws Exception {
+	public void resolvePreferredNotSupported() {
 		this.resolver.setSupportedLocales(Collections.singletonList(CANADA));
 		assertEquals(US, this.resolver.resolveLocaleContext(exchange(US, UK)).getLocale());
 	}
@@ -61,14 +63,65 @@ public class AcceptHeaderLocaleContextResolverTests {
 	public void resolvePreferredNotSupportedWithDefault() {
 		this.resolver.setSupportedLocales(Arrays.asList(US, JAPAN));
 		this.resolver.setDefaultLocale(JAPAN);
-
+		assertEquals(JAPAN, this.resolver.resolveLocaleContext(exchange(KOREA)).getLocale());
-		MockServerHttpRequest request = MockServerHttpRequest.get("/").acceptLanguageAsLocales(KOREA).build();
-		MockServerWebExchange exchange = MockServerWebExchange.from(request);
-		assertEquals(JAPAN, this.resolver.resolveLocaleContext(exchange).getLocale());
 	}
 
 	@Test
-	public void defaultLocale() throws Exception {
+	public void resolvePreferredAgainstLanguageOnly() {
+		this.resolver.setSupportedLocales(Collections.singletonList(ENGLISH));
+		assertEquals(ENGLISH, this.resolver.resolveLocaleContext(exchange(GERMANY, US, UK)).getLocale());
+	}
+
+	@Test
+	public void resolveMissingAcceptLanguageHeader() {
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		assertNull(this.resolver.resolveLocaleContext(exchange).getLocale());
+	}
+
+	@Test
+	public void resolveMissingAcceptLanguageHeaderWithDefault() {
+		this.resolver.setDefaultLocale(US);
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		assertEquals(US, this.resolver.resolveLocaleContext(exchange).getLocale());
+	}
+
+	@Test
+	public void resolveEmptyAcceptLanguageHeader() {
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").header(HttpHeaders.ACCEPT_LANGUAGE, "").build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		assertNull(this.resolver.resolveLocaleContext(exchange).getLocale());
+	}
+
+	@Test
+	public void resolveEmptyAcceptLanguageHeaderWithDefault() {
+		this.resolver.setDefaultLocale(US);
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").header(HttpHeaders.ACCEPT_LANGUAGE, "").build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		assertEquals(US, this.resolver.resolveLocaleContext(exchange).getLocale());
+	}
+
+	@Test
+	public void resolveInvalidAcceptLanguageHeader() {
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").header(HttpHeaders.ACCEPT_LANGUAGE, "en_US").build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		assertNull(this.resolver.resolveLocaleContext(exchange).getLocale());
+	}
+
+	@Test
+	public void resolveInvalidAcceptLanguageHeaderWithDefault() {
+		this.resolver.setDefaultLocale(US);
+
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").header(HttpHeaders.ACCEPT_LANGUAGE, "en_US").build();
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		assertEquals(US, this.resolver.resolveLocaleContext(exchange).getLocale());
+	}
+
+	@Test
+	public void defaultLocale() {
 		this.resolver.setDefaultLocale(JAPANESE);
 		MockServerHttpRequest request = MockServerHttpRequest.get("/").build();
 		MockServerWebExchange exchange = MockServerWebExchange.from(request);

spring-web/src/test/java/org/springframework/web/server/i18n/FixedLocaleContextResolverTests.java

@@ -1,3 +1,19 @@
+/*
+ * Copyright 2002-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package org.springframework.web.server.i18n;
 
 import java.time.ZoneId;
@@ -12,10 +28,8 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.web.server.ServerWebExchange;
 
-import static java.util.Locale.CANADA;
+import static java.util.Locale.*;
-import static java.util.Locale.FRANCE;
+import static org.junit.Assert.*;
-import static java.util.Locale.US;
-import static org.junit.Assert.assertEquals;
 
 /**
  * Unit tests for {@link FixedLocaleContextResolver}.
@@ -24,8 +38,6 @@ import static org.junit.Assert.assertEquals;
  */
 public class FixedLocaleContextResolverTests {
 
-	private FixedLocaleContextResolver resolver;
-
 	@Before
 	public void setup() {
 		Locale.setDefault(US);
@@ -33,23 +45,23 @@ public class FixedLocaleContextResolverTests {
 
 	@Test
 	public void resolveDefaultLocale() {
-		this.resolver = new FixedLocaleContextResolver();
+		FixedLocaleContextResolver resolver = new FixedLocaleContextResolver();
-		assertEquals(US, this.resolver.resolveLocaleContext(exchange()).getLocale());
+		assertEquals(US, resolver.resolveLocaleContext(exchange()).getLocale());
-		assertEquals(US, this.resolver.resolveLocaleContext(exchange(CANADA)).getLocale());
+		assertEquals(US, resolver.resolveLocaleContext(exchange(CANADA)).getLocale());
 	}
 
 	@Test
 	public void resolveCustomizedLocale() {
-		this.resolver = new FixedLocaleContextResolver(FRANCE);
+		FixedLocaleContextResolver resolver = new FixedLocaleContextResolver(FRANCE);
-		assertEquals(FRANCE, this.resolver.resolveLocaleContext(exchange()).getLocale());
+		assertEquals(FRANCE, resolver.resolveLocaleContext(exchange()).getLocale());
-		assertEquals(FRANCE, this.resolver.resolveLocaleContext(exchange(CANADA)).getLocale());
+		assertEquals(FRANCE, resolver.resolveLocaleContext(exchange(CANADA)).getLocale());
 	}
 
 	@Test
 	public void resolveCustomizedAndTimeZoneLocale() {
 		TimeZone timeZone = TimeZone.getTimeZone(ZoneId.of("UTC"));
-		this.resolver = new FixedLocaleContextResolver(FRANCE, timeZone);
+		FixedLocaleContextResolver resolver = new FixedLocaleContextResolver(FRANCE, timeZone);
-		TimeZoneAwareLocaleContext context = (TimeZoneAwareLocaleContext)this.resolver.resolveLocaleContext(exchange());
+		TimeZoneAwareLocaleContext context = (TimeZoneAwareLocaleContext) resolver.resolveLocaleContext(exchange());
 		assertEquals(FRANCE, context.getLocale());
 		assertEquals(timeZone, context.getTimeZone());
 	}

spring-webmvc/src/main/java/org/springframework/web/servlet/i18n/AcceptHeaderLocaleResolver.java

@@ -55,11 +55,9 @@ public class AcceptHeaderLocaleResolver implements LocaleResolver {
 	 * @param locales the supported locales
 	 * @since 4.3
 	 */
-	public void setSupportedLocales(@Nullable List<Locale> locales) {
+	public void setSupportedLocales(List<Locale> locales) {
 		this.supportedLocales.clear();
-		if (locales != null) {
+		this.supportedLocales.addAll(locales);
-			this.supportedLocales.addAll(locales);
-		}
 	}
 
 	/**