diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java index 3fa2f5198e..52cf8ea526 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.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. @@ -167,41 +167,48 @@ public @interface RequestMapping { String[] headers() default {}; /** - * The consumable media types of the mapped request, narrowing the primary mapping. - *

The format is a single media type or a sequence of media types, - * with a request only mapped if the {@code Content-Type} matches one of these media types. - * Examples: + * Narrows the primary mapping by media types that can be consumed by the + * mapped handler. Consists of one or more media types one of which must + * match to the request {@code Content-Type} header. Examples: *

 	 * consumes = "text/plain"
 	 * consumes = {"text/plain", "application/*"}
+	 * consumes = MediaType.TEXT_PLAIN_VALUE
 	 * 
- * 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". + * 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". *

Supported at the type level as well as at the method level! - * When used at the type level, all method-level mappings override - * this consumes restriction. + * If specified at both levels, the method level consumes condition overrides + * the type level condition. * @see org.springframework.http.MediaType * @see javax.servlet.http.HttpServletRequest#getContentType() */ String[] consumes() default {}; /** - * The producible media types of the mapped request, narrowing the primary mapping. - *

The format is a single media type or a sequence of media types, - * with a request only mapped if the {@code Accept} matches one of these media types. - * Examples: + * Narrows the primary mapping by media types that can be produced by the + * mapped handler. Consists of one or more media types one of which must + * be chosen via content negotiation against the "acceptable" media types + * of the request. Typically those are extracted from the {@code "Accept"} + * header but may be derived from query parameters, or other. Examples: *

 	 * produces = "text/plain"
 	 * produces = {"text/plain", "application/*"}
-	 * produces = MediaType.APPLICATION_JSON_UTF8_VALUE
+	 * produces = MediaType.TEXT_PLAIN_VALUE
+	 * produces = "text/plain;charset=UTF-8"
 	 * 
- *

It affects the actual content type written, for example to produce a JSON response - * with UTF-8 encoding, {@link org.springframework.http.MediaType#APPLICATION_JSON_UTF8_VALUE} should be used. - *

Expressions can be negated by using the "!" operator, as in "!text/plain", which matches - * all requests with a {@code Accept} other than "text/plain". + *

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 + * if the media type from the request does not contain the parameter, it is + * assumed the client accepts any value. + *

Expressions can be negated by using the "!" operator, as in "!text/plain", + * which matches all requests with a {@code Accept} other than "text/plain". *

Supported at the type level as well as at the method level! - * When used at the type level, all method-level mappings override - * this produces restriction. + * If specified at both levels, the method level produces condition overrides + * the type level condition. + * @see org.springframework.http.MediaType * @see org.springframework.http.MediaType */ String[] produces() default {}; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java index fd8c00790d..a90597c372 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java @@ -25,6 +25,7 @@ import java.util.Set; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; +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; @@ -315,12 +316,23 @@ public final class ProducesRequestCondition extends AbstractRequestCondition acceptedMediaTypes = getAcceptedMediaTypes(exchange); for (MediaType acceptedMediaType : acceptedMediaTypes) { - if (getMediaType().isCompatibleWith(acceptedMediaType)) { + if (getMediaType().isCompatibleWith(acceptedMediaType) && matchParameters(acceptedMediaType)) { return true; } } 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; + } } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java index c4d35c09e1..7abe05d674 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java @@ -24,12 +24,8 @@ import org.junit.Test; import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.web.server.ServerWebExchange; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get; +import static org.junit.Assert.*; +import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*; /** * Unit tests for {@link ProducesRequestCondition}. @@ -84,6 +80,29 @@ public class ProducesRequestConditionTests { assertNull(condition.getMatchingCondition(exchange)); } + @Test // gh-21670 + 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")); + assertNotNull("Declared parameter value must match if present in request", + condition.getMatchingCondition(exchange)); + + condition = new ProducesRequestCondition(base + ";type=feed"); + exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=entry")); + assertNull("Declared parameter value must match if present in request", + condition.getMatchingCondition(exchange)); + + condition = new ProducesRequestCondition(base + ";type=feed"); + exchange = MockServerWebExchange.from(get("/").header("Accept", base)); + assertNotNull("Declared parameter has no impact if not present in request", + condition.getMatchingCondition(exchange)); + + condition = new ProducesRequestCondition(base); + exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=feed")); + assertNotNull("No impact from other parameters in request", condition.getMatchingCondition(exchange)); + } + @Test public void matchParseError() { MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "bogus")); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java index 8aa4c0274a..3f41cd2430 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java @@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; import org.springframework.web.HttpMediaTypeException; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.accept.ContentNegotiationManager; @@ -322,12 +323,23 @@ public final class ProducesRequestCondition extends AbstractRequestCondition acceptedMediaTypes) { for (MediaType acceptedMediaType : acceptedMediaTypes) { - if (getMediaType().isCompatibleWith(acceptedMediaType)) { + if (getMediaType().isCompatibleWith(acceptedMediaType) && matchParameters(acceptedMediaType)) { return true; } } 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; + } } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java index 1c50b5f606..fa0c3946cd 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java @@ -90,6 +90,30 @@ public class ProducesRequestConditionTests { assertNull(condition.getMatchingCondition(request)); } + @Test // gh-21670 + public void matchWithParameters() { + String base = "application/atom+xml"; + ProducesRequestCondition condition = new ProducesRequestCondition(base + ";type=feed"); + HttpServletRequest request = createRequest(base + ";type=entry"); + assertNull("Declared parameter value must match if present in request", + condition.getMatchingCondition(request)); + + condition = new ProducesRequestCondition(base + ";type=feed"); + request = createRequest(base + ";type=feed"); + assertNotNull("Declared parameter value must match if present in request", + condition.getMatchingCondition(request)); + + condition = new ProducesRequestCondition(base + ";type=feed"); + request = createRequest(base); + assertNotNull("Declared parameter has no impact if not present in request", + condition.getMatchingCondition(request)); + + condition = new ProducesRequestCondition(base); + request = createRequest(base + ";type=feed"); + assertNotNull("No impact from other parameters in request", + condition.getMatchingCondition(request)); + } + @Test public void matchParseError() { ProducesRequestCondition condition = new ProducesRequestCondition("text/plain");