Consistent support for multiple Accept headers

Issue: SPR-14506

Author: jhoeller

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

@@ -429,19 +429,7 @@ public void setAccept(List<MediaType> acceptableMediaTypes) {
 	 * <p>Returns an empty list when the acceptable media types are unspecified.
 	 */
 	public List<MediaType> getAccept() {
-		String value = getFirst(ACCEPT);
-		List<MediaType> result = (value != null ? MediaType.parseMediaTypes(value) : Collections.<MediaType>emptyList());
-
-		// Some containers parse 'Accept' into multiple values
-		if (result.size() == 1) {
-			List<String> acceptHeader = get(ACCEPT);
-			if (acceptHeader.size() > 1) {
-				value = StringUtils.collectionToCommaDelimitedString(acceptHeader);
-				result = MediaType.parseMediaTypes(value);
-			}
-		}
-
-		return result;
+		return MediaType.parseMediaTypes(get(ACCEPT));
 	}
 
 	/**
@@ -452,7 +440,7 @@ public void setAccessControlAllowCredentials(boolean allowCredentials) {
 	}
 
 	/**
-	 * Returns the value of the {@code Access-Control-Allow-Credentials} response header.
+	 * Return the value of the {@code Access-Control-Allow-Credentials} response header.
 	 */
 	public boolean getAccessControlAllowCredentials() {
 		return Boolean.parseBoolean(getFirst(ACCESS_CONTROL_ALLOW_CREDENTIALS));
@@ -466,7 +454,7 @@ public void setAccessControlAllowHeaders(List<String> allowedHeaders) {
 	}
 
 	/**
-	 * Returns the value of the {@code Access-Control-Allow-Headers} response header.
+	 * Return the value of the {@code Access-Control-Allow-Headers} response header.
 	 */
 	public List<String> getAccessControlAllowHeaders() {
 		return getValuesAsList(ACCESS_CONTROL_ALLOW_HEADERS);
@@ -519,7 +507,7 @@ public void setAccessControlExposeHeaders(List<String> exposedHeaders) {
 	}
 
 	/**
-	 * Returns the value of the {@code Access-Control-Expose-Headers} response header.
+	 * Return the value of the {@code Access-Control-Expose-Headers} response header.
 	 */
 	public List<String> getAccessControlExposeHeaders() {
 		return getValuesAsList(ACCESS_CONTROL_EXPOSE_HEADERS);
@@ -533,7 +521,7 @@ public void setAccessControlMaxAge(long maxAge) {
 	}
 
 	/**
-	 * Returns the value of the {@code Access-Control-Max-Age} response header.
+	 * Return the value of the {@code Access-Control-Max-Age} response header.
 	 * <p>Returns -1 when the max age is unknown.
 	 */
 	public long getAccessControlMaxAge() {
@@ -549,7 +537,7 @@ public void setAccessControlRequestHeaders(List<String> requestHeaders) {
 	}
 
 	/**
-	 * Returns the value of the {@code Access-Control-Request-Headers} request header.
+	 * Return the value of the {@code Access-Control-Request-Headers} request header.
 	 */
 	public List<String> getAccessControlRequestHeaders() {
 		return getValuesAsList(ACCESS_CONTROL_REQUEST_HEADERS);

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

@@ -28,6 +28,7 @@
 import java.util.stream.Collectors;
 
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.InvalidMimeTypeException;
 import org.springframework.util.MimeType;
 import org.springframework.util.MimeTypeUtils;
@@ -397,6 +398,8 @@ public MediaType removeQualityValue() {
 	 * Parse the given String value into a {@code MediaType} object,
 	 * with this method name following the 'valueOf' naming convention
 	 * (as supported by {@link org.springframework.core.convert.ConversionService}.
+	 * @param value the string to parse
+	 * @throws InvalidMediaTypeException if the media type value cannot be parsed
 	 * @see #parseMediaType(String)
 	 */
 	public static MediaType valueOf(String value) {
@@ -407,7 +410,7 @@ public static MediaType valueOf(String value) {
 	 * Parse the given String into a single {@code MediaType}.
 	 * @param mediaType the string to parse
 	 * @return the media type
-	 * @throws InvalidMediaTypeException if the string cannot be parsed
+	 * @throws InvalidMediaTypeException if the media type value cannot be parsed
 	 */
 	public static MediaType parseMediaType(String mediaType) {
 		MimeType type;
@@ -425,13 +428,12 @@ public static MediaType parseMediaType(String mediaType) {
 		}
 	}
 
-
 	/**
-	 * Parse the given, comma-separated string into a list of {@code MediaType} objects.
+	 * Parse the given comma-separated string into a list of {@code MediaType} objects.
 	 * <p>This method can be used to parse an Accept or Content-Type header.
 	 * @param mediaTypes the string to parse
 	 * @return the list of media types
-	 * @throws IllegalArgumentException if the string cannot be parsed
+	 * @throws InvalidMediaTypeException if the media type value cannot be parsed
 	 */
 	public static List<MediaType> parseMediaTypes(String mediaTypes) {
 		if (!StringUtils.hasLength(mediaTypes)) {
@@ -445,6 +447,31 @@ public static List<MediaType> parseMediaTypes(String mediaTypes) {
 		return result;
 	}
 
+	/**
+	 * Parse the given list of (potentially) comma-separated strings into a
+	 * list of {@code MediaType} objects.
+	 * <p>This method can be used to parse an Accept or Content-Type header.
+	 * @param mediaTypes the string to parse
+	 * @return the list of media types
+	 * @throws InvalidMediaTypeException if the media type value cannot be parsed
+	 * @since 4.3.2
+	 */
+	public static List<MediaType> parseMediaTypes(List<String> mediaTypes) {
+		if (CollectionUtils.isEmpty(mediaTypes)) {
+			return Collections.<MediaType>emptyList();
+		}
+		else if (mediaTypes.size() == 1) {
+			return parseMediaTypes(mediaTypes.get(0));
+		}
+		else {
+			List<MediaType> result = new ArrayList<>(8);
+			for (String mediaType : mediaTypes) {
+				result.addAll(parseMediaTypes(mediaType));
+			}
+			return result;
+		}
+	}
+
 	/**
 	 * Re-create the given mime types as media types.
 	 * @since 5.0

spring-web/src/main/java/org/springframework/web/accept/HeaderContentNegotiationStrategy.java

@@ -16,20 +16,21 @@
 
 package org.springframework.web.accept;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.InvalidMediaTypeException;
 import org.springframework.http.MediaType;
-import org.springframework.util.StringUtils;
 import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.context.request.NativeWebRequest;
 
 /**
  * A {@code ContentNegotiationStrategy} that checks the 'Accept' request header.
  *
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  * @since 3.2
  */
 public class HeaderContentNegotiationStrategy implements ContentNegotiationStrategy {
@@ -42,18 +43,20 @@ public class HeaderContentNegotiationStrategy implements ContentNegotiationStrat
 	public List<MediaType> resolveMediaTypes(NativeWebRequest request)
 			throws HttpMediaTypeNotAcceptableException {
 
-		String header = request.getHeader(HttpHeaders.ACCEPT);
-		if (!StringUtils.hasText(header)) {
-			return Collections.emptyList();
+		String[] headerValueArray = request.getHeaderValues(HttpHeaders.ACCEPT);
+		if (headerValueArray == null) {
+			return Collections.<MediaType>emptyList();
 		}
+
+		List<String> headerValues = Arrays.asList(headerValueArray);
 		try {
-			List<MediaType> mediaTypes = MediaType.parseMediaTypes(header);
+			List<MediaType> mediaTypes = MediaType.parseMediaTypes(headerValues);
 			MediaType.sortBySpecificityAndQuality(mediaTypes);
 			return mediaTypes;
 		}
 		catch (InvalidMediaTypeException ex) {
 			throw new HttpMediaTypeNotAcceptableException(
-					"Could not parse 'Accept' header [" + header + "]: " + ex.getMessage());
+					"Could not parse 'Accept' header " + headerValues + ": " + ex.getMessage());
 		}
 	}
 

spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java

@@ -68,13 +68,22 @@ public void accept() {
 	}
 
 	@Test  // SPR-9655
-	public void acceptIPlanet() {
+	public void acceptWithMultipleHeaderValues() {
 		headers.add("Accept", "text/html");
 		headers.add("Accept", "text/plain");
 		List<MediaType> expected = Arrays.asList(new MediaType("text", "html"), new MediaType("text", "plain"));
 		assertEquals("Invalid Accept header", expected, headers.getAccept());
 	}
 
+	@Test  // SPR-14506
+	public void acceptWithMultipleCommaSeparatedHeaderValues() {
+		headers.add("Accept", "text/html,text/pdf");
+		headers.add("Accept", "text/plain,text/csv");
+		List<MediaType> expected = Arrays.asList(new MediaType("text", "html"), new MediaType("text", "pdf"),
+				new MediaType("text", "plain"), new MediaType("text", "csv"));
+		assertEquals("Invalid Accept header", expected, headers.getAccept());
+	}
+
 	@Test
 	public void acceptCharsets() {
 		Charset charset1 = StandardCharsets.UTF_8;

spring-web/src/test/java/org/springframework/http/MediaTypeTests.java

@@ -138,7 +138,7 @@ public void parseMediaTypes() throws Exception {
 		assertNotNull("No media types returned", mediaTypes);
 		assertEquals("Invalid amount of media types", 4, mediaTypes.size());
 
-		mediaTypes = MediaType.parseMediaTypes(null);
+		mediaTypes = MediaType.parseMediaTypes("");
 		assertNotNull("No media types returned", mediaTypes);
 		assertEquals("Invalid amount of media types", 0, mediaTypes.size());
 	}

spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -13,11 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.web.accept;
 
 import java.util.List;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.http.MediaType;
@@ -32,21 +32,16 @@
  * Test fixture for HeaderContentNegotiationStrategy tests.
  *
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  */
 public class HeaderContentNegotiationStrategyTests {
 
-	private HeaderContentNegotiationStrategy strategy;
+	private final HeaderContentNegotiationStrategy strategy = new HeaderContentNegotiationStrategy();
 
-	private NativeWebRequest webRequest;
+	private final MockHttpServletRequest servletRequest = new MockHttpServletRequest();
 
-	private MockHttpServletRequest servletRequest;
+	private final NativeWebRequest webRequest = new ServletWebRequest(this.servletRequest);
 
-	@Before
-	public void setup() {
-		this.strategy = new HeaderContentNegotiationStrategy();
-		this.servletRequest = new MockHttpServletRequest();
-		this.webRequest = new ServletWebRequest(servletRequest );
-	}
 
 	@Test
 	public void resolveMediaTypes() throws Exception {
@@ -60,7 +55,20 @@ public void resolveMediaTypes() throws Exception {
 		assertEquals("text/plain;q=0.5", mediaTypes.get(3).toString());
 	}
 
-	@Test(expected=HttpMediaTypeNotAcceptableException.class)
+	@Test  // SPR-14506
+	public void resolveMediaTypesFromMultipleHeaderValues() throws Exception {
+		this.servletRequest.addHeader("Accept", "text/plain; q=0.5, text/html");
+		this.servletRequest.addHeader("Accept", "text/x-dvi; q=0.8, text/x-c");
+		List<MediaType> mediaTypes = this.strategy.resolveMediaTypes(this.webRequest);
+
+		assertEquals(4, mediaTypes.size());
+		assertEquals("text/html", mediaTypes.get(0).toString());
+		assertEquals("text/x-c", mediaTypes.get(1).toString());
+		assertEquals("text/x-dvi;q=0.8", mediaTypes.get(2).toString());
+		assertEquals("text/plain;q=0.5", mediaTypes.get(3).toString());
+	}
+
+	@Test(expected = HttpMediaTypeNotAcceptableException.class)
 	public void resolveMediaTypesParseError() throws Exception {
 		this.servletRequest.addHeader("Accept", "textplain; q=0.5");
 		this.strategy.resolveMediaTypes(this.webRequest);