More accurate checks for presence of MediaType.ALL

Typically a straight up equals as well as Collections#contains
checks for MediaType.ALL is susceptible to the presence of
media type parameters.

This commits adds equalsTypeAndSubtype as well as an
isPresentIn(Collection<MimeType>) methods to MimeType to faciliate
with checks for MediaType.ALL.

Issue: SPR-17550

spring-core/src/main/java/org/springframework/util/MimeType.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -19,6 +19,7 @@ package org.springframework.util;
 import java.io.Serializable;
 import java.nio.charset.Charset;
 import java.util.BitSet;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Iterator;
@@ -383,6 +384,37 @@ public class MimeType implements Comparable<MimeType>, Serializable {
 		return false;
 	}
 
+	/**
+	 * Similar to {@link #equals(Object)} but based on the type and subtype
+	 * only, i.e. ignoring parameters.
+	 * @param other the other mime type to compare to
+	 * @return whether the two mime types have the same type and subtype
+	 * @since 5.1.4
+	 */
+	public boolean equalsTypeAndSubtype(@Nullable MimeType other) {
+		if (other == null) {
+			return false;
+		}
+		return this.type.equalsIgnoreCase(other.type) && this.subtype.equalsIgnoreCase(other.subtype);
+	}
+
+	/**
+	 * Unlike {@link Collection#contains(Object)} which relies on
+	 * {@link MimeType#equals(Object)}, this method only checks the type and the
+	 * subtype, but otherwise ignores parameters.
+	 * @param mimeTypes the list of mime types to perform the check against
+	 * @return whether the list contains the given mime type
+	 * @since 5.1.4
+	 */
+	public boolean isPresentIn(Collection<? extends MimeType> mimeTypes) {
+		for (MimeType mimeType : mimeTypes) {
+			if (mimeType.equalsTypeAndSubtype(this)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 
 	@Override
 	public boolean equals(Object other) {

spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -177,7 +177,7 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
 	 * or if the media type is {@code null}
 	 */
 	protected boolean canWrite(@Nullable MediaType mediaType) {
-		if (mediaType == null || MediaType.ALL.equals(mediaType)) {
+		if (mediaType == null || MediaType.ALL.equalsTypeAndSubtype(mediaType)) {
 			return true;
 		}
 		for (MediaType supportedMediaType : getSupportedMediaTypes()) {

spring-web/src/main/java/org/springframework/http/converter/BufferedImageHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -150,7 +150,7 @@ public class BufferedImageHttpMessageConverter implements HttpMessageConverter<B
 	}
 
 	private boolean isWritable(@Nullable MediaType mediaType) {
-		if (mediaType == null || MediaType.ALL.equals(mediaType)) {
+		if (mediaType == null || MediaType.ALL.equalsTypeAndSubtype(mediaType)) {
 			return true;
 		}
 		Iterator<ImageWriter> imageWriters = ImageIO.getImageWritersByMIMEType(mediaType.toString());

spring-web/src/main/java/org/springframework/http/converter/ByteArrayHttpMessageConverter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -42,7 +42,7 @@ public class ByteArrayHttpMessageConverter extends AbstractHttpMessageConverter<
 	 * Create a new instance of the {@code ByteArrayHttpMessageConverter}.
 	 */
 	public ByteArrayHttpMessageConverter() {
-		super(new MediaType("application", "octet-stream"), MediaType.ALL);
+		super(MediaType.APPLICATION_OCTET_STREAM, MediaType.ALL);
 	}
 
 

spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -17,6 +17,7 @@
 package org.springframework.web.reactive.result;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Comparator;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -47,7 +48,8 @@ import org.springframework.web.server.ServerWebExchange;
  */
 public abstract class HandlerResultHandlerSupport implements Ordered {
 
-	private static final MediaType MEDIA_TYPE_APPLICATION_ALL = new MediaType("application");
+	private static final List<MediaType> ALL_APPLICATION_MEDIA_TYPES =
+			Arrays.asList(MediaType.ALL, new MediaType("application"));
 
 
 	protected final Log logger = LogFactory.getLog(getClass());
@@ -147,7 +149,7 @@ public abstract class HandlerResultHandlerSupport implements Ordered {
 				selected = mediaType;
 				break;
 			}
-			else if (mediaType.equals(MediaType.ALL) || mediaType.equals(MEDIA_TYPE_APPLICATION_ALL)) {
+			else if (mediaType.isPresentIn(ALL_APPLICATION_MEDIA_TYPES)) {
 				selected = MediaType.APPLICATION_OCTET_STREAM;
 				break;
 			}

spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -33,6 +33,7 @@ import org.springframework.web.reactive.accept.RequestedContentTypeResolver;
 import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder;
 import org.springframework.web.server.NotAcceptableStatusException;
 import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.server.UnsupportedMediaTypeStatusException;
 
 /**
  * A logical disjunction (' || ') request condition to match a request's 'Accept' header
@@ -48,6 +49,8 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
 
 	private static final ProducesRequestCondition PRE_FLIGHT_MATCH = new ProducesRequestCondition();
 
+	private static final ProducesRequestCondition EMPTY_CONDITION = new ProducesRequestCondition();
+
 
 	private final List<ProduceMediaTypeExpression> mediaTypeAllList =
 			Collections.singletonList(new ProduceMediaTypeExpression(MediaType.ALL_VALUE));
@@ -192,7 +195,20 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
 		}
 		Set<ProduceMediaTypeExpression> result = new LinkedHashSet<>(this.expressions);
 		result.removeIf(expression -> !expression.match(exchange));
-		return (!result.isEmpty() ? new ProducesRequestCondition(result, this.contentTypeResolver) : null);
+		if (!result.isEmpty()) {
+			return new ProducesRequestCondition(result, this.contentTypeResolver);
+		}
+		else {
+			try {
+				if (MediaType.ALL.isPresentIn(getAcceptedMediaTypes(exchange))) {
+					return EMPTY_CONDITION;
+				}
+			}
+			catch (NotAcceptableStatusException | UnsupportedMediaTypeStatusException ex) {
+				// Ignore
+			}
+		}
+		return null;
 	}
 
 	/**

spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -39,7 +39,7 @@ import static org.springframework.mock.http.server.reactive.test.MockServerHttpR
 public class ProducesRequestConditionTests {
 
 	@Test
-	public void match() throws Exception {
+	public void match() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/plain"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("text/plain");
 
@@ -47,7 +47,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void matchNegated() throws Exception {
+	public void matchNegated() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/plain"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("!text/plain");
 
@@ -55,13 +55,13 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void getProducibleMediaTypes() throws Exception {
+	public void getProducibleMediaTypes() {
 		ProducesRequestCondition condition = new ProducesRequestCondition("!application/xml");
 		assertEquals(Collections.emptySet(), condition.getProducibleMediaTypes());
 	}
 
 	@Test
-	public void matchWildcard() throws Exception {
+	public void matchWildcard() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/plain"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("text/*");
 
@@ -69,7 +69,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void matchMultiple() throws Exception {
+	public void matchMultiple() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/plain"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("text/plain", "application/xml");
 
@@ -77,7 +77,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void matchSingle() throws Exception {
+	public void matchSingle() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "application/xml"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("text/plain");
 
@@ -85,7 +85,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void matchParseError() throws Exception {
+	public void matchParseError() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "bogus"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("text/plain");
 
@@ -93,15 +93,26 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void matchParseErrorWithNegation() throws Exception {
+	public void matchParseErrorWithNegation() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "bogus"));
 		ProducesRequestCondition condition = new ProducesRequestCondition("!text/plain");
 
 		assertNull(condition.getMatchingCondition(exchange));
 	}
 
+	@Test // SPR-17550
+	public void matchWithNegationAndMediaTypeAllWithQualityParameter() {
+		ProducesRequestCondition condition = new ProducesRequestCondition("!application/json");
+
+		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept",
+				"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8"));
+
+		assertNotNull(condition.getMatchingCondition(exchange));
+	}
+
+
 	@Test
-	public void compareTo() throws Exception {
+	public void compareTo() {
 		ProducesRequestCondition html = new ProducesRequestCondition("text/html");
 		ProducesRequestCondition xml = new ProducesRequestCondition("application/xml");
 		ProducesRequestCondition none = new ProducesRequestCondition();
@@ -136,7 +147,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void compareToWithSingleExpression() throws Exception {
+	public void compareToWithSingleExpression() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/plain"));
 
 		ProducesRequestCondition condition1 = new ProducesRequestCondition("text/plain");
@@ -150,7 +161,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void compareToMultipleExpressions() throws Exception {
+	public void compareToMultipleExpressions() {
 		ProducesRequestCondition condition1 = new ProducesRequestCondition("*/*", "text/plain");
 		ProducesRequestCondition condition2 = new ProducesRequestCondition("text/*", "text/plain;q=0.7");
 
@@ -164,7 +175,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void compareToMultipleExpressionsAndMultipleAcceptHeaderValues() throws Exception {
+	public void compareToMultipleExpressionsAndMultipleAcceptHeaderValues() {
 		ProducesRequestCondition condition1 = new ProducesRequestCondition("text/*", "text/plain");
 		ProducesRequestCondition condition2 = new ProducesRequestCondition("application/*", "application/xml");
 
@@ -190,7 +201,7 @@ public class ProducesRequestConditionTests {
 	// SPR-8536
 
 	@Test
-	public void compareToMediaTypeAll() throws Exception {
+	public void compareToMediaTypeAll() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/"));
 
 		ProducesRequestCondition condition1 = new ProducesRequestCondition();
@@ -226,7 +237,7 @@ public class ProducesRequestConditionTests {
 	// SPR-9021
 
 	@Test
-	public void compareToMediaTypeAllWithParameter() throws Exception {
+	public void compareToMediaTypeAllWithParameter() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "*/*;q=0.9"));
 
 		ProducesRequestCondition condition1 = new ProducesRequestCondition();
@@ -237,7 +248,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void compareToEqualMatch() throws Exception {
+	public void compareToEqualMatch() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/*"));
 
 		ProducesRequestCondition condition1 = new ProducesRequestCondition("text/plain");
@@ -251,7 +262,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void combine() throws Exception {
+	public void combine() {
 		ProducesRequestCondition condition1 = new ProducesRequestCondition("text/plain");
 		ProducesRequestCondition condition2 = new ProducesRequestCondition("application/xml");
 
@@ -260,7 +271,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void combineWithDefault() throws Exception {
+	public void combineWithDefault() {
 		ProducesRequestCondition condition1 = new ProducesRequestCondition("text/plain");
 		ProducesRequestCondition condition2 = new ProducesRequestCondition();
 
@@ -269,7 +280,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void instantiateWithProducesAndHeaderConditions() throws Exception {
+	public void instantiateWithProducesAndHeaderConditions() {
 		String[] produces = new String[] {"text/plain"};
 		String[] headers = new String[]{"foo=bar", "accept=application/xml,application/pdf"};
 		ProducesRequestCondition condition = new ProducesRequestCondition(produces, headers);
@@ -278,7 +289,7 @@ public class ProducesRequestConditionTests {
 	}
 
 	@Test
-	public void getMatchingCondition() throws Exception {
+	public void getMatchingCondition() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "text/plain"));
 
 		ProducesRequestCondition condition = new ProducesRequestCondition("text/plain", "application/xml");

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -207,7 +207,7 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
 		if (!result.isEmpty()) {
 			return new ProducesRequestCondition(result, this.contentNegotiationManager);
 		}
-		else if (acceptedMediaTypes.contains(MediaType.ALL)) {
+		else if (MediaType.ALL.isPresentIn(acceptedMediaTypes)) {
 			return EMPTY_CONDITION;
 		}
 		else {

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -82,7 +82,8 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 	private static final Set<String> WHITELISTED_MEDIA_BASE_TYPES = new HashSet<>(
 			Arrays.asList("audio", "image", "video"));
 
-	private static final MediaType MEDIA_TYPE_APPLICATION = new MediaType("application");
+	private static final List<MediaType> ALL_APPLICATION_MEDIA_TYPES =
+			Arrays.asList(MediaType.ALL, new MediaType("application"));
 
 	private static final Type RESOURCE_REGION_LIST_TYPE =
 			new ParameterizedTypeReference<List<ResourceRegion>>() { }.getType();
@@ -257,7 +258,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 					selectedMediaType = mediaType;
 					break;
 				}
-				else if (mediaType.equals(MediaType.ALL) || mediaType.equals(MEDIA_TYPE_APPLICATION)) {
+				else if (mediaType.isPresentIn(ALL_APPLICATION_MEDIA_TYPES)) {
 					selectedMediaType = MediaType.APPLICATION_OCTET_STREAM;
 					break;
 				}

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -128,6 +128,17 @@ public class ProducesRequestConditionTests {
 		assertNotNull(condition.getMatchingCondition(request));
 	}
 
+	@Test // SPR-17550
+	public void matchWithNegationAndMediaTypeAllWithQualityParameter() {
+		ProducesRequestCondition condition = new ProducesRequestCondition("!application/json");
+
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addHeader("Accept",
+				"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8");
+
+		assertNotNull(condition.getMatchingCondition(request));
+	}
+
 	@Test
 	public void compareTo() {
 		ProducesRequestCondition html = new ProducesRequestCondition("text/html");