Port fix for SPR-14397

This is a port of the following commit, adapted for Java 8+:
89396ff01f
This commit is contained in:
Rossen Stoyanchev 2016-06-29 17:34:45 -04:00
parent 478b4149f7
commit e9d8152ab2
2 changed files with 214 additions and 133 deletions

View File

@ -20,7 +20,6 @@ import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
@ -35,18 +34,16 @@ import org.springframework.http.HttpMethod;
import org.springframework.http.InvalidMediaTypeException;
import org.springframework.http.MediaType;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.util.CollectionUtils;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.reactive.HandlerMapping;
import org.springframework.web.reactive.result.condition.ParamsRequestCondition;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.reactive.result.condition.NameValueExpression;
import org.springframework.web.server.MethodNotAllowedException;
import org.springframework.web.server.NotAcceptableStatusException;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.server.UnsupportedMediaTypeStatusException;
/**
@ -207,59 +204,29 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
* method but not by query parameter conditions
*/
@Override
protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> requestMappingInfos,
String lookupPath, ServerWebExchange exchange) throws Exception {
protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> infos, String lookupPath,
ServerWebExchange exchange) throws Exception {
Set<String> allowedMethods = new LinkedHashSet<>(4);
PartialMatchHelper helper = new PartialMatchHelper(infos, exchange);
Set<RequestMappingInfo> patternMatches = new HashSet<>();
Set<RequestMappingInfo> patternAndMethodMatches = new HashSet<>();
for (RequestMappingInfo info : requestMappingInfos) {
if (info.getPatternsCondition().getMatchingCondition(exchange) != null) {
patternMatches.add(info);
if (info.getMethodsCondition().getMatchingCondition(exchange) != null) {
patternAndMethodMatches.add(info);
}
else {
for (RequestMethod method : info.getMethodsCondition().getMethods()) {
allowedMethods.add(method.name());
}
}
}
if (helper.isEmpty()) {
return null;
}
ServerHttpRequest request = exchange.getRequest();
if (patternMatches.isEmpty()) {
return null;
}
else if (patternAndMethodMatches.isEmpty()) {
if (helper.hasMethodsMismatch()) {
HttpMethod httpMethod = request.getMethod();
Set<String> methods = helper.getAllowedMethods();
if (HttpMethod.OPTIONS.matches(httpMethod.name())) {
HttpOptionsHandler handler = new HttpOptionsHandler(allowedMethods);
HttpOptionsHandler handler = new HttpOptionsHandler(methods);
return new HandlerMethod(handler, HTTP_OPTIONS_HANDLE_METHOD);
}
else if (!allowedMethods.isEmpty()) {
throw new MethodNotAllowedException(httpMethod.name(), allowedMethods);
}
throw new MethodNotAllowedException(httpMethod.name(), methods);
}
Set<MediaType> consumableMediaTypes;
Set<MediaType> producibleMediaTypes;
List<List<String>> paramConditions;
if (patternAndMethodMatches.isEmpty()) {
consumableMediaTypes = getConsumableMediaTypes(exchange, patternMatches);
producibleMediaTypes = getProducibleMediaTypes(exchange, patternMatches);
paramConditions = getRequestParams(exchange, patternMatches);
}
else {
consumableMediaTypes = getConsumableMediaTypes(exchange, patternAndMethodMatches);
producibleMediaTypes = getProducibleMediaTypes(exchange, patternAndMethodMatches);
paramConditions = getRequestParams(exchange, patternAndMethodMatches);
}
if (!consumableMediaTypes.isEmpty()) {
if (helper.hasConsumesMismatch()) {
Set<MediaType> mediaTypes = helper.getConsumableMediaTypes();
MediaType contentType;
try {
contentType = request.getHeaders().getContentType();
@ -267,62 +234,176 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
catch (InvalidMediaTypeException ex) {
throw new UnsupportedMediaTypeStatusException(ex.getMessage());
}
throw new UnsupportedMediaTypeStatusException(contentType, new ArrayList<>(consumableMediaTypes));
throw new UnsupportedMediaTypeStatusException(contentType, new ArrayList<>(mediaTypes));
}
else if (!producibleMediaTypes.isEmpty()) {
throw new NotAcceptableStatusException(new ArrayList<>(producibleMediaTypes));
if (helper.hasProducesMismatch()) {
Set<MediaType> mediaTypes = helper.getProducibleMediaTypes();
throw new NotAcceptableStatusException(new ArrayList<>(mediaTypes));
}
else {
if (!CollectionUtils.isEmpty(paramConditions)) {
Map<String, String[]> params = request.getQueryParams().entrySet().stream()
.collect(Collectors.toMap(Entry::getKey,
entry -> entry.getValue().toArray(new String[entry.getValue().size()]))
);
throw new ServerWebInputException("Unsatisfied query parameter conditions: " +
paramConditions + ", actual parameters: " + params);
if (helper.hasParamsMismatch()) {
throw new ServerWebInputException(
"Unsatisfied query parameter conditions: " + helper.getParamConditions() +
", actual parameters: " + request.getQueryParams());
}
return null;
}
/**
* Aggregate all partial matches and expose methods checking across them.
*/
private static class PartialMatchHelper {
private final List<PartialMatch> partialMatches = new ArrayList<>();
public PartialMatchHelper(Set<RequestMappingInfo> infos, ServerWebExchange exchange) {
this.partialMatches.addAll(infos.stream().
filter(info -> info.getPatternsCondition().getMatchingCondition(exchange) != null).
map(info -> new PartialMatch(info, exchange)).
collect(Collectors.toList()));
}
/**
* Whether there any partial matches.
*/
public boolean isEmpty() {
return this.partialMatches.isEmpty();
}
/**
* Any partial matches for "methods"?
*/
public boolean hasMethodsMismatch() {
return !this.partialMatches.stream().
filter(PartialMatch::hasMethodsMatch).findAny().isPresent();
}
/**
* Any partial matches for "methods" and "consumes"?
*/
public boolean hasConsumesMismatch() {
return !this.partialMatches.stream().
filter(PartialMatch::hasConsumesMatch).findAny().isPresent();
}
/**
* Any partial matches for "methods", "consumes", and "produces"?
*/
public boolean hasProducesMismatch() {
return !this.partialMatches.stream().
filter(PartialMatch::hasProducesMatch).findAny().isPresent();
}
/**
* Any partial matches for "methods", "consumes", "produces", and "params"?
*/
public boolean hasParamsMismatch() {
return !this.partialMatches.stream().
filter(PartialMatch::hasParamsMatch).findAny().isPresent();
}
/**
* Return declared HTTP methods.
*/
public Set<String> getAllowedMethods() {
return this.partialMatches.stream().
flatMap(m -> m.getInfo().getMethodsCondition().getMethods().stream()).
map(Enum::name).
collect(Collectors.toCollection(LinkedHashSet::new));
}
/**
* Return declared "consumable" types but only among those that also
* match the "methods" condition.
*/
public Set<MediaType> getConsumableMediaTypes() {
return this.partialMatches.stream().filter(PartialMatch::hasMethodsMatch).
flatMap(m -> m.getInfo().getConsumesCondition().getConsumableMediaTypes().stream()).
collect(Collectors.toCollection(LinkedHashSet::new));
}
/**
* Return declared "producible" types but only among those that also
* match the "methods" and "consumes" conditions.
*/
public Set<MediaType> getProducibleMediaTypes() {
return this.partialMatches.stream().filter(PartialMatch::hasConsumesMatch).
flatMap(m -> m.getInfo().getProducesCondition().getProducibleMediaTypes().stream()).
collect(Collectors.toCollection(LinkedHashSet::new));
}
/**
* Return declared "params" conditions but only among those that also
* match the "methods", "consumes", and "params" conditions.
*/
public List<Set<NameValueExpression<String>>> getParamConditions() {
return this.partialMatches.stream().filter(PartialMatch::hasProducesMatch).
map(match -> match.getInfo().getParamsCondition().getExpressions()).
collect(Collectors.toList());
}
/**
* Container for a RequestMappingInfo that matches the URL path at least.
*/
private static class PartialMatch {
private final RequestMappingInfo info;
private final boolean methodsMatch;
private final boolean consumesMatch;
private final boolean producesMatch;
private final boolean paramsMatch;
/**
* @param info RequestMappingInfo that matches the URL path
* @param exchange the current exchange
*/
public PartialMatch(RequestMappingInfo info, ServerWebExchange exchange) {
this.info = info;
this.methodsMatch = info.getMethodsCondition().getMatchingCondition(exchange) != null;
this.consumesMatch = info.getConsumesCondition().getMatchingCondition(exchange) != null;
this.producesMatch = info.getProducesCondition().getMatchingCondition(exchange) != null;
this.paramsMatch = info.getParamsCondition().getMatchingCondition(exchange) != null;
}
else {
return null;
public RequestMappingInfo getInfo() {
return this.info;
}
public boolean hasMethodsMatch() {
return this.methodsMatch;
}
public boolean hasConsumesMatch() {
return hasMethodsMatch() && this.consumesMatch;
}
public boolean hasProducesMatch() {
return hasConsumesMatch() && this.producesMatch;
}
public boolean hasParamsMatch() {
return hasProducesMatch() && this.paramsMatch;
}
@Override
public String toString() {
return this.info.toString();
}
}
}
private Set<MediaType> getConsumableMediaTypes(ServerWebExchange exchange,
Set<RequestMappingInfo> partialMatches) {
Set<MediaType> result = new HashSet<>();
for (RequestMappingInfo partialMatch : partialMatches) {
if (partialMatch.getConsumesCondition().getMatchingCondition(exchange) == null) {
result.addAll(partialMatch.getConsumesCondition().getConsumableMediaTypes());
}
}
return result;
}
private Set<MediaType> getProducibleMediaTypes(ServerWebExchange exchange,
Set<RequestMappingInfo> partialMatches) {
Set<MediaType> result = new HashSet<>();
for (RequestMappingInfo partialMatch : partialMatches) {
if (partialMatch.getProducesCondition().getMatchingCondition(exchange) == null) {
result.addAll(partialMatch.getProducesCondition().getProducibleMediaTypes());
}
}
return result;
}
private List<List<String>> getRequestParams(ServerWebExchange exchange,
Set<RequestMappingInfo> partialMatches) {
return partialMatches.stream()
.map(RequestMappingInfo::getParamsCondition)
.filter(condition -> condition.getMatchingCondition(exchange) == null)
.map(ParamsRequestCondition::getExpressions)
.map(expressions -> expressions.stream().map(Object::toString).collect(Collectors.toList()))
.collect(Collectors.toList());
}
/**
* Default handler for HTTP OPTIONS.
*/

View File

@ -32,6 +32,7 @@ import org.junit.Test;
import reactor.core.publisher.Mono;
import reactor.core.test.TestSubscriber;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
@ -43,6 +44,8 @@ import org.springframework.stereotype.Controller;
import org.springframework.ui.ExtendedModelMap;
import org.springframework.ui.ModelMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
@ -50,18 +53,20 @@ import org.springframework.web.method.HandlerMethod;
import org.springframework.web.reactive.HandlerMapping;
import org.springframework.web.reactive.HandlerResult;
import org.springframework.web.reactive.result.method.RequestMappingInfo.BuilderConfiguration;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.server.MethodNotAllowedException;
import org.springframework.web.server.NotAcceptableStatusException;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.server.UnsupportedMediaTypeStatusException;
import org.springframework.web.server.adapter.DefaultServerWebExchange;
import org.springframework.web.server.session.WebSessionManager;
import org.springframework.web.util.HttpRequestPathHelper;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
@ -147,41 +152,28 @@ public class RequestMappingInfoHandlerMappingTests {
public void getHandlerRequestMethodNotAllowed() throws Exception {
ServerWebExchange exchange = createExchange(HttpMethod.POST, "/bar");
Mono<Object> mono = this.handlerMapping.getHandler(exchange);
assertError(mono, MethodNotAllowedException.class,
ex -> assertEquals(new HashSet<>(Arrays.asList("GET", "HEAD")), ex.getSupportedMethods()));
}
// SPR-9603
@Test
@Test // SPR-9603
public void getHandlerRequestMethodMatchFalsePositive() throws Exception {
ServerWebExchange exchange = createExchange(HttpMethod.GET, "/users");
exchange.getRequest().getHeaders().setAccept(Collections.singletonList(MediaType.APPLICATION_XML));
this.handlerMapping.registerHandler(new UserController());
Mono<Object> mono = this.handlerMapping.getHandler(exchange);
TestSubscriber
.subscribe(mono)
.assertError(NotAcceptableStatusException.class);
TestSubscriber.subscribe(mono).assertError(NotAcceptableStatusException.class);
}
// SPR-8462
@Test
@Test // SPR-8462
public void getHandlerMediaTypeNotSupported() throws Exception {
testHttpMediaTypeNotSupportedException("/person/1");
testHttpMediaTypeNotSupportedException("/person/1/");
testHttpMediaTypeNotSupportedException("/person/1.json");
}
@Test
public void getHandlerHttpOptions() throws Exception {
testHttpOptions("/foo", "GET,HEAD");
testHttpOptions("/person/1", "PUT");
testHttpOptions("/persons", "GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS");
testHttpOptions("/something", "PUT,POST");
}
@Test
public void getHandlerTestInvalidContentType() throws Exception {
ServerWebExchange exchange = createExchange(HttpMethod.PUT, "/person/1");
@ -196,24 +188,32 @@ public class RequestMappingInfoHandlerMappingTests {
// SPR-8462
@Test
public void getHandlerMediaTypeNotAccepted() throws Exception {
testHttpMediaTypeNotAcceptableException("/persons");
testHttpMediaTypeNotAcceptableException("/persons/");
testHttpMediaTypeNotAcceptableException("/persons.json");
public void getHandlerTestMediaTypeNotAcceptable() throws Exception {
testMediaTypeNotAcceptable("/persons");
testMediaTypeNotAcceptable("/persons/");
testMediaTypeNotAcceptable("/persons.json");
}
// SPR-12854
@Test
public void getHandlerUnsatisfiedServletRequestParameterException() throws Exception {
public void getHandlerTestRequestParamMismatch() throws Exception {
ServerWebExchange exchange = createExchange(HttpMethod.GET, "/params");
Mono<Object> mono = this.handlerMapping.getHandler(exchange);
assertError(mono, ServerWebInputException.class, ex -> {
assertEquals(ex.getReason(), "Unsatisfied query parameter conditions: " +
"[[bar=baz], [foo=bar]], actual parameters: {}");
assertThat(ex.getReason(), containsString("[foo=bar]"));
assertThat(ex.getReason(), containsString("[bar=baz]"));
});
}
@Test
public void getHandlerHttpOptions() throws Exception {
testHttpOptions("/foo", "GET,HEAD");
testHttpOptions("/person/1", "PUT");
testHttpOptions("/persons", "GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS");
testHttpOptions("/something", "PUT,POST");
}
@Test
public void getHandlerProducibleMediaTypesAttribute() throws Exception {
ServerWebExchange exchange = createExchange(HttpMethod.GET, "/content");
@ -398,7 +398,7 @@ public class RequestMappingInfoHandlerMappingTests {
assertEquals(allowHeader, ((HttpHeaders) value.get()).getFirst("Allow"));
}
private void testHttpMediaTypeNotAcceptableException(String url) throws Exception {
private void testMediaTypeNotAcceptable(String url) throws Exception {
ServerWebExchange exchange = createExchange(HttpMethod.GET, url);
exchange.getRequest().getHeaders().setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
Mono<Object> mono = this.handlerMapping.getHandler(exchange);
@ -431,15 +431,15 @@ public class RequestMappingInfoHandlerMappingTests {
@Controller
private static class TestController {
@RequestMapping(value = "/foo", method = RequestMethod.GET)
@GetMapping("/foo")
public void foo() {
}
@RequestMapping(value = "/foo", method = RequestMethod.GET, params="p")
@GetMapping(path = "/foo", params="p")
public void fooParam() {
}
@RequestMapping(value = "/ba*", method = { RequestMethod.GET, RequestMethod.HEAD })
@RequestMapping(path = "/ba*", method = { RequestMethod.GET, RequestMethod.HEAD })
public void bar() {
}
@ -447,7 +447,7 @@ public class RequestMappingInfoHandlerMappingTests {
public void empty() {
}
@RequestMapping(value = "/person/{id}", method = RequestMethod.PUT, consumes="application/xml")
@PutMapping(path = "/person/{id}", consumes="application/xml")
public void consumes(@RequestBody String text) {
}
@ -488,11 +488,11 @@ public class RequestMappingInfoHandlerMappingTests {
@Controller
private static class UserController {
@RequestMapping(value = "/users", method = RequestMethod.GET, produces = "application/json")
@GetMapping(path = "/users", produces = "application/json")
public void getUser() {
}
@RequestMapping(value = "/users", method = RequestMethod.PUT)
@PutMapping(path = "/users")
public void saveUser() {
}
}
@ -510,16 +510,16 @@ public class RequestMappingInfoHandlerMappingTests {
@Override
protected RequestMappingInfo getMappingForMethod(Method method, Class<?> handlerType) {
RequestMapping annotation = AnnotationUtils.findAnnotation(method, RequestMapping.class);
if (annotation != null) {
RequestMapping annot = AnnotatedElementUtils.findMergedAnnotation(method, RequestMapping.class);
if (annot != null) {
BuilderConfiguration options = new BuilderConfiguration();
options.setPathHelper(getPathHelper());
options.setPathMatcher(getPathMatcher());
options.setSuffixPatternMatch(true);
options.setTrailingSlashMatch(true);
return RequestMappingInfo.paths(annotation.value()).methods(annotation.method())
.params(annotation.params()).headers(annotation.headers())
.consumes(annotation.consumes()).produces(annotation.produces())
return RequestMappingInfo.paths(annot.value()).methods(annot.method())
.params(annot.params()).headers(annot.headers())
.consumes(annot.consumes()).produces(annot.produces())
.options(options).build();
}
else {