The "consumes" condition compares MediaType parameters

Closes gh-9257

spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -169,7 +169,12 @@
 	 * consumes = {"text/plain", "application/*"}
 	 * consumes = MediaType.TEXT_PLAIN_VALUE
 	 * </pre>
-	 * Expressions can be negated by using the "!" operator, as in
+	 * <p>If a declared media type contains a parameter, and if the
+	 * {@code "content-type"} from the request also has that parameter, then
+	 * the parameter values must match. Otherwise, if the media type from the
+	 * request {@code "content-type"} does not contain the parameter, then the
+	 * parameter is ignored for matching purposes.
+	 * <p>Expressions can be negated by using the "!" operator, as in
 	 * "!text/plain", which matches all requests with a {@code Content-Type}
 	 * other than "text/plain".
 	 * <p><b>Supported at the type level as well as at the method level!</b>
@@ -194,7 +199,7 @@
 	 * </pre>
 	 * <p>If a declared media type contains a parameter (e.g. "charset=UTF-8",
 	 * "type=feed", "type=entry") and if a compatible media type from the request
-	 * has that parameter too, then the parameter values must match. Otherwise
+	 * has that parameter too, then the parameter values must match. Otherwise,
 	 * if the media type from the request does not contain the parameter, it is
 	 * assumed the client accepts any value.
 	 * <p>Expressions can be negated by using the "!" operator, as in "!text/plain",

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -16,8 +16,11 @@
 
 package org.springframework.web.reactive.result.condition;
 
+import java.util.Map;
+
 import org.springframework.http.MediaType;
 import org.springframework.lang.Nullable;
+import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.server.NotAcceptableStatusException;
 import org.springframework.web.server.ServerWebExchange;
@@ -78,6 +81,18 @@ public final boolean match(ServerWebExchange exchange) {
 	protected abstract boolean matchMediaType(ServerWebExchange exchange)
 			throws NotAcceptableStatusException, UnsupportedMediaTypeStatusException;
 
+	protected boolean matchParameters(MediaType contentType) {
+		for (Map.Entry<String, String> entry : getMediaType().getParameters().entrySet()) {
+			if (StringUtils.hasText(entry.getValue())) {
+				String value = contentType.getParameter(entry.getKey());
+				if (StringUtils.hasText(value) && !entry.getValue().equals(value)) {
+					return false;
+				}
+			}
+		}
+		return true;
+	}
+
 	@Override
 	public int compareTo(AbstractMediaTypeExpression other) {
 		MediaType mediaType1 = this.getMediaType();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -274,7 +274,7 @@ protected boolean matchMediaType(ServerWebExchange exchange) throws UnsupportedM
 			try {
 				MediaType contentType = exchange.getRequest().getHeaders().getContentType();
 				contentType = (contentType != null ? contentType : MediaType.APPLICATION_OCTET_STREAM);
-				return getMediaType().includes(contentType);
+				return (getMediaType().includes(contentType) && matchParameters(contentType));
 			}
 			catch (InvalidMediaTypeException ex) {
 				throw new UnsupportedMediaTypeStatusException("Can't parse Content-Type [" +

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -27,7 +27,6 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.MimeType;
 import org.springframework.util.ObjectUtils;
-import org.springframework.util.StringUtils;
 import org.springframework.web.accept.ContentNegotiationManager;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.cors.reactive.CorsUtils;
@@ -361,17 +360,6 @@ protected boolean matchMediaType(ServerWebExchange exchange) throws NotAcceptabl
 			}
 			return false;
 		}
-
-		private boolean matchParameters(MediaType acceptedMediaType) {
-			for (String name : getMediaType().getParameters().keySet()) {
-				String s1 = getMediaType().getParameter(name);
-				String s2 = acceptedMediaType.getParameter(name);
-				if (StringUtils.hasText(s1) && StringUtils.hasText(s2) && !s1.equalsIgnoreCase(s2)) {
-					return false;
-				}
-			}
-			return true;
-		}
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -35,61 +35,81 @@
 public class ConsumesRequestConditionTests {
 
 	@Test
-	public void consumesMatch() throws Exception {
+	public void consumesMatch() {
 		MockServerWebExchange exchange = postExchange("text/plain");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("text/plain");
 
 		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
 	}
 
 	@Test
-	public void negatedConsumesMatch() throws Exception {
+	public void negatedConsumesMatch() {
 		MockServerWebExchange exchange = postExchange("text/plain");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("!text/plain");
 
 		assertThat(condition.getMatchingCondition(exchange)).isNull();
 	}
 
 	@Test
-	public void getConsumableMediaTypesNegatedExpression() throws Exception {
+	public void getConsumableMediaTypesNegatedExpression() {
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("!application/xml");
 		assertThat(condition.getConsumableMediaTypes()).isEqualTo(Collections.emptySet());
 	}
 
 	@Test
-	public void consumesWildcardMatch() throws Exception {
+	public void consumesWildcardMatch() {
 		MockServerWebExchange exchange = postExchange("text/plain");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("text/*");
 
 		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
 	}
 
 	@Test
-	public void consumesMultipleMatch() throws Exception {
+	public void consumesMultipleMatch() {
 		MockServerWebExchange exchange = postExchange("text/plain");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("text/plain", "application/xml");
 
 		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
 	}
 
 	@Test
-	public void consumesSingleNoMatch() throws Exception {
+	public void consumesSingleNoMatch() {
 		MockServerWebExchange exchange = postExchange("application/xml");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("text/plain");
 
 		assertThat(condition.getMatchingCondition(exchange)).isNull();
 	}
 
+	@Test // gh-28024
+	public void matchWithParameters() {
+		String base = "application/hal+json";
+		ConsumesRequestCondition condition = new ConsumesRequestCondition(base + ";profile=\"a\"");
+		MockServerWebExchange exchange = postExchange(base + ";profile=\"a\"");
+		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
+
+		condition = new ConsumesRequestCondition(base + ";profile=\"a\"");
+		exchange = postExchange(base + ";profile=\"b\"");
+		assertThat(condition.getMatchingCondition(exchange)).isNull();
+
+		condition = new ConsumesRequestCondition(base + ";profile=\"a\"");
+		exchange = postExchange(base);
+		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
+
+		condition = new ConsumesRequestCondition(base);
+		exchange = postExchange(base + ";profile=\"a\"");
+		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
+	}
+
 	@Test
-	public void consumesParseError() throws Exception {
+	public void consumesParseError() {
 		MockServerWebExchange exchange = postExchange("01");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("text/plain");
 
 		assertThat(condition.getMatchingCondition(exchange)).isNull();
 	}
 
 	@Test
-	public void consumesParseErrorWithNegation() throws Exception {
+	public void consumesParseErrorWithNegation() {
 		MockServerWebExchange exchange = postExchange("01");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("!text/plain");
 
@@ -115,7 +135,7 @@ public void consumesNoContent() {
 	}
 
 	@Test
-	public void compareToSingle() throws Exception {
+	public void compareToSingle() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
 
 		ConsumesRequestCondition condition1 = new ConsumesRequestCondition("text/plain");
@@ -129,7 +149,7 @@ public void compareToSingle() throws Exception {
 	}
 
 	@Test
-	public void compareToMultiple() throws Exception {
+	public void compareToMultiple() {
 		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
 
 		ConsumesRequestCondition condition1 = new ConsumesRequestCondition("*/*", "text/plain");
@@ -153,7 +173,7 @@ public void combine() throws Exception {
 	}
 
 	@Test
-	public void combineWithDefault() throws Exception {
+	public void combineWithDefault() {
 		ConsumesRequestCondition condition1 = new ConsumesRequestCondition("text/plain");
 		ConsumesRequestCondition condition2 = new ConsumesRequestCondition();
 
@@ -162,7 +182,7 @@ public void combineWithDefault() throws Exception {
 	}
 
 	@Test
-	public void parseConsumesAndHeaders() throws Exception {
+	public void parseConsumesAndHeaders() {
 		String[] consumes = new String[] {"text/plain"};
 		String[] headers = new String[]{"foo=bar", "content-type=application/xml,application/pdf"};
 		ConsumesRequestCondition condition = new ConsumesRequestCondition(consumes, headers);
@@ -171,7 +191,7 @@ public void parseConsumesAndHeaders() throws Exception {
 	}
 
 	@Test
-	public void getMatchingCondition() throws Exception {
+	public void getMatchingCondition() {
 		MockServerWebExchange exchange = postExchange("text/plain");
 		ConsumesRequestCondition condition = new ConsumesRequestCondition("text/plain", "application/xml");
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -89,19 +89,19 @@ public void matchWithParameters() {
 		String base = "application/atom+xml";
 		ProducesRequestCondition condition = new ProducesRequestCondition(base + ";type=feed");
 		MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=feed"));
-		assertThat(condition.getMatchingCondition(exchange)).as("Declared parameter value must match if present in request").isNotNull();
+		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
 
 		condition = new ProducesRequestCondition(base + ";type=feed");
 		exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=entry"));
-		assertThat(condition.getMatchingCondition(exchange)).as("Declared parameter value must match if present in request").isNull();
+		assertThat(condition.getMatchingCondition(exchange)).isNull();
 
 		condition = new ProducesRequestCondition(base + ";type=feed");
 		exchange = MockServerWebExchange.from(get("/").header("Accept", base));
-		assertThat(condition.getMatchingCondition(exchange)).as("Declared parameter has no impact if not present in request").isNotNull();
+		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
 
 		condition = new ProducesRequestCondition(base);
 		exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=feed"));
-		assertThat(condition.getMatchingCondition(exchange)).as("No impact from other parameters in request").isNotNull();
+		assertThat(condition.getMatchingCondition(exchange)).isNotNull();
 	}
 
 	@Test

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -16,8 +16,11 @@
 
 package org.springframework.web.servlet.mvc.condition;
 
+import java.util.Map;
+
 import org.springframework.http.MediaType;
 import org.springframework.lang.Nullable;
+import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.RequestMapping;
 
 /**
@@ -78,6 +81,18 @@ else if (mediaType1.isLessSpecific(mediaType2)) {
 		}
 	}
 
+	protected boolean matchParameters(MediaType contentType) {
+		for (Map.Entry<String, String> entry : getMediaType().getParameters().entrySet()) {
+			if (StringUtils.hasText(entry.getValue())) {
+				String value = contentType.getParameter(entry.getKey());
+				if (StringUtils.hasText(value) && !entry.getValue().equals(value)) {
+					return false;
+				}
+			}
+		}
+		return true;
+	}
+
 	@Override
 	public boolean equals(@Nullable Object other) {
 		if (this == other) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -285,7 +285,7 @@ static class ConsumeMediaTypeExpression extends AbstractMediaTypeExpression {
 		}
 
 		public final boolean match(MediaType contentType) {
-			boolean match = getMediaType().includes(contentType);
+			boolean match = (getMediaType().includes(contentType) && matchParameters(contentType));
 			return !isNegated() == match;
 		}
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -29,7 +29,6 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.MimeType;
 import org.springframework.util.ObjectUtils;
-import org.springframework.util.StringUtils;
 import org.springframework.web.HttpMediaTypeException;
 import org.springframework.web.HttpMediaTypeNotAcceptableException;
 import org.springframework.web.accept.ContentNegotiationManager;
@@ -373,17 +372,6 @@ private boolean matchMediaType(List<MediaType> acceptedMediaTypes) {
 			}
 			return false;
 		}
-
-		private boolean matchParameters(MediaType acceptedMediaType) {
-			for (String name : getMediaType().getParameters().keySet()) {
-				String s1 = getMediaType().getParameter(name);
-				String s2 = acceptedMediaType.getParameter(name);
-				if (StringUtils.hasText(s1) && StringUtils.hasText(s2) && !s1.equalsIgnoreCase(s2)) {
-					return false;
-				}
-			}
-			return true;
-		}
 	}
 
 }