Skip to content

Commit 8dc535c

Browse files
rstoyanchevrstoyanchev
committed
Match declared parameters on produces condition
Closes gh-21670
1 parent c0be1c5 commit 8dc535c

File tree

5 files changed

+102
-28
lines changed

5 files changed

+102
-28
lines changed

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

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -167,41 +167,48 @@
167167
String[] headers() default {};
168168

169169
/**
170-
* The consumable media types of the mapped request, narrowing the primary mapping.
171-
* <p>The format is a single media type or a sequence of media types,
172-
* with a request only mapped if the {@code Content-Type} matches one of these media types.
173-
* Examples:
170+
* Narrows the primary mapping by media types that can be consumed by the
171+
* mapped handler. Consists of one or more media types one of which must
172+
* match to the request {@code Content-Type} header. Examples:
174173
* <pre class="code">
175174
* consumes = "text/plain"
176175
* consumes = {"text/plain", "application/*"}
176+
* consumes = MediaType.TEXT_PLAIN_VALUE
177177
* </pre>
178-
* Expressions can be negated by using the "!" operator, as in "!text/plain", which matches
179-
* all requests with a {@code Content-Type} other than "text/plain".
178+
* Expressions can be negated by using the "!" operator, as in
179+
* "!text/plain", which matches all requests with a {@code Content-Type}
180+
* other than "text/plain".
180181
* <p><b>Supported at the type level as well as at the method level!</b>
181-
* When used at the type level, all method-level mappings override
182-
* this consumes restriction.
182+
* If specified at both levels, the method level consumes condition overrides
183+
* the type level condition.
183184
* @see org.springframework.http.MediaType
184185
* @see javax.servlet.http.HttpServletRequest#getContentType()
185186
*/
186187
String[] consumes() default {};
187188

188189
/**
189-
* The producible media types of the mapped request, narrowing the primary mapping.
190-
* <p>The format is a single media type or a sequence of media types,
191-
* with a request only mapped if the {@code Accept} matches one of these media types.
192-
* Examples:
190+
* Narrows the primary mapping by media types that can be produced by the
191+
* mapped handler. Consists of one or more media types one of which must
192+
* be chosen via content negotiation against the "acceptable" media types
193+
* of the request. Typically those are extracted from the {@code "Accept"}
194+
* header but may be derived from query parameters, or other. Examples:
193195
* <pre class="code">
194196
* produces = "text/plain"
195197
* produces = {"text/plain", "application/*"}
196-
* produces = MediaType.APPLICATION_JSON_UTF8_VALUE
198+
* produces = MediaType.TEXT_PLAIN_VALUE
199+
* produces = "text/plain;charset=UTF-8"
197200
* </pre>
198-
* <p>It affects the actual content type written, for example to produce a JSON response
199-
* with UTF-8 encoding, {@link org.springframework.http.MediaType#APPLICATION_JSON_UTF8_VALUE} should be used.
200-
* <p>Expressions can be negated by using the "!" operator, as in "!text/plain", which matches
201-
* all requests with a {@code Accept} other than "text/plain".
201+
* <p>If a declared media type contains a parameter (e.g. "charset=UTF-8",
202+
* "type=feed", type="entry") and if a compatible media type from the request
203+
* has that parameter too, then the parameter values must match. Otherwise
204+
* if the media type from the request does not contain the parameter, it is
205+
* assumed the client accepts any value.
206+
* <p>Expressions can be negated by using the "!" operator, as in "!text/plain",
207+
* which matches all requests with a {@code Accept} other than "text/plain".
202208
* <p><b>Supported at the type level as well as at the method level!</b>
203-
* When used at the type level, all method-level mappings override
204-
* this produces restriction.
209+
* If specified at both levels, the method level produces condition overrides
210+
* the type level condition.
211+
* @see org.springframework.http.MediaType
205212
* @see org.springframework.http.MediaType
206213
*/
207214
String[] produces() default {};

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import org.springframework.http.MediaType;
2727
import org.springframework.lang.Nullable;
28+
import org.springframework.util.StringUtils;
2829
import org.springframework.web.accept.ContentNegotiationManager;
2930
import org.springframework.web.bind.annotation.RequestMapping;
3031
import org.springframework.web.cors.reactive.CorsUtils;
@@ -315,12 +316,23 @@ class ProduceMediaTypeExpression extends AbstractMediaTypeExpression {
315316
protected boolean matchMediaType(ServerWebExchange exchange) throws NotAcceptableStatusException {
316317
List<MediaType> acceptedMediaTypes = getAcceptedMediaTypes(exchange);
317318
for (MediaType acceptedMediaType : acceptedMediaTypes) {
318-
if (getMediaType().isCompatibleWith(acceptedMediaType)) {
319+
if (getMediaType().isCompatibleWith(acceptedMediaType) && matchParameters(acceptedMediaType)) {
319320
return true;
320321
}
321322
}
322323
return false;
323324
}
325+
326+
private boolean matchParameters(MediaType acceptedMediaType) {
327+
for (String name : getMediaType().getParameters().keySet()) {
328+
String s1 = getMediaType().getParameter(name);
329+
String s2 = acceptedMediaType.getParameter(name);
330+
if (StringUtils.hasText(s1) && StringUtils.hasText(s2) && !s1.equalsIgnoreCase(s2)) {
331+
return false;
332+
}
333+
}
334+
return true;
335+
}
324336
}
325337

326338
}

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

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,8 @@
2424
import org.springframework.mock.web.test.server.MockServerWebExchange;
2525
import org.springframework.web.server.ServerWebExchange;
2626

27-
import static org.junit.Assert.assertEquals;
28-
import static org.junit.Assert.assertNotNull;
29-
import static org.junit.Assert.assertNull;
30-
import static org.junit.Assert.assertTrue;
31-
import static org.junit.Assert.fail;
32-
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get;
27+
import static org.junit.Assert.*;
28+
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
3329

3430
/**
3531
* Unit tests for {@link ProducesRequestCondition}.
@@ -84,6 +80,29 @@ public void matchSingle() {
8480
assertNull(condition.getMatchingCondition(exchange));
8581
}
8682

83+
@Test // gh-21670
84+
public void matchWithParameters() {
85+
String base = "application/atom+xml";
86+
ProducesRequestCondition condition = new ProducesRequestCondition(base + ";type=feed");
87+
MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=feed"));
88+
assertNotNull("Declared parameter value must match if present in request",
89+
condition.getMatchingCondition(exchange));
90+
91+
condition = new ProducesRequestCondition(base + ";type=feed");
92+
exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=entry"));
93+
assertNull("Declared parameter value must match if present in request",
94+
condition.getMatchingCondition(exchange));
95+
96+
condition = new ProducesRequestCondition(base + ";type=feed");
97+
exchange = MockServerWebExchange.from(get("/").header("Accept", base));
98+
assertNotNull("Declared parameter has no impact if not present in request",
99+
condition.getMatchingCondition(exchange));
100+
101+
condition = new ProducesRequestCondition(base);
102+
exchange = MockServerWebExchange.from(get("/").header("Accept", base + ";type=feed"));
103+
assertNotNull("No impact from other parameters in request", condition.getMatchingCondition(exchange));
104+
}
105+
87106
@Test
88107
public void matchParseError() {
89108
MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "bogus"));

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.springframework.http.MediaType;
2828
import org.springframework.lang.Nullable;
29+
import org.springframework.util.StringUtils;
2930
import org.springframework.web.HttpMediaTypeException;
3031
import org.springframework.web.HttpMediaTypeNotAcceptableException;
3132
import org.springframework.web.accept.ContentNegotiationManager;
@@ -322,12 +323,23 @@ public final boolean match(List<MediaType> acceptedMediaTypes) {
322323

323324
private boolean matchMediaType(List<MediaType> acceptedMediaTypes) {
324325
for (MediaType acceptedMediaType : acceptedMediaTypes) {
325-
if (getMediaType().isCompatibleWith(acceptedMediaType)) {
326+
if (getMediaType().isCompatibleWith(acceptedMediaType) && matchParameters(acceptedMediaType)) {
326327
return true;
327328
}
328329
}
329330
return false;
330331
}
332+
333+
private boolean matchParameters(MediaType acceptedMediaType) {
334+
for (String name : getMediaType().getParameters().keySet()) {
335+
String s1 = getMediaType().getParameter(name);
336+
String s2 = acceptedMediaType.getParameter(name);
337+
if (StringUtils.hasText(s1) && StringUtils.hasText(s2) && !s1.equalsIgnoreCase(s2)) {
338+
return false;
339+
}
340+
}
341+
return true;
342+
}
331343
}
332344

333345
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,30 @@ public void matchSingle() {
9090
assertNull(condition.getMatchingCondition(request));
9191
}
9292

93+
@Test // gh-21670
94+
public void matchWithParameters() {
95+
String base = "application/atom+xml";
96+
ProducesRequestCondition condition = new ProducesRequestCondition(base + ";type=feed");
97+
HttpServletRequest request = createRequest(base + ";type=entry");
98+
assertNull("Declared parameter value must match if present in request",
99+
condition.getMatchingCondition(request));
100+
101+
condition = new ProducesRequestCondition(base + ";type=feed");
102+
request = createRequest(base + ";type=feed");
103+
assertNotNull("Declared parameter value must match if present in request",
104+
condition.getMatchingCondition(request));
105+
106+
condition = new ProducesRequestCondition(base + ";type=feed");
107+
request = createRequest(base);
108+
assertNotNull("Declared parameter has no impact if not present in request",
109+
condition.getMatchingCondition(request));
110+
111+
condition = new ProducesRequestCondition(base);
112+
request = createRequest(base + ";type=feed");
113+
assertNotNull("No impact from other parameters in request",
114+
condition.getMatchingCondition(request));
115+
}
116+
93117
@Test
94118
public void matchParseError() {
95119
ProducesRequestCondition condition = new ProducesRequestCondition("text/plain");

0 commit comments

Comments
 (0)