From 3272a3b8badc29fd6f6022476dfb0124fbfeca85 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 30 Jun 2015 15:24:01 -0400 Subject: [PATCH] Check HTTP method before raising 415 This commit moves the check whether an HTTP method supports request body up to the base class so that all sub-classes can benefit (not just @RequestBody). Issue: SPR-13176 --- ...MessageConverterMethodArgumentResolver.java | 17 +++++++++++++++++ .../RequestResponseBodyMethodProcessor.java | 13 +++++-------- .../HttpEntityMethodProcessorMockTests.java | 15 +++++++-------- .../HttpEntityMethodProcessorTests.java | 18 +++++++++++------- ...tMappingHandlerAdapterIntegrationTests.java | 11 ++++++++++- 5 files changed, 50 insertions(+), 24 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java index fef30d6685..f4e62129c8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java @@ -22,6 +22,7 @@ import java.io.PushbackInputStream; import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; @@ -37,6 +38,8 @@ import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.http.converter.GenericHttpMessageConverter; @@ -60,6 +63,9 @@ import org.springframework.web.method.support.HandlerMethodArgumentResolver; */ public abstract class AbstractMessageConverterMethodArgumentResolver implements HandlerMethodArgumentResolver { + private static final List SUPPORTED_METHODS = + Arrays.asList(HttpMethod.POST, HttpMethod.PUT, HttpMethod.PATCH); + private static final Object NO_VALUE = new Object(); @@ -170,6 +176,7 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements targetClass = (Class) resolvableType.resolve(); } + HttpMethod httpMethod = ((HttpRequest) inputMessage).getMethod(); inputMessage = new EmptyBodyCheckingHttpInputMessage(inputMessage); Object body = NO_VALUE; @@ -213,6 +220,9 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements } if (body == NO_VALUE) { + if (!SUPPORTED_METHODS.contains(httpMethod)) { + return null; + } throw new HttpMediaTypeNotSupportedException(contentType, this.allSupportedMediaTypes); } @@ -273,6 +283,8 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements private final InputStream body; + private final HttpMethod method; + public EmptyBodyCheckingHttpInputMessage(HttpInputMessage inputMessage) throws IOException { this.headers = inputMessage.getHeaders(); @@ -296,6 +308,7 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements pushbackInputStream.unread(b); } } + this.method = ((HttpRequest) inputMessage).getMethod(); } @Override @@ -307,6 +320,10 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements public InputStream getBody() throws IOException { return this.body; } + + public HttpMethod getMethod() { + return this.method; + } } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java index 979a3aa49b..48552d7202 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java @@ -150,14 +150,11 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter ServletServerHttpRequest inputMessage = new ServletServerHttpRequest(servletRequest); Object arg = null; - if (webRequest.getHeader("Content-Type") != null || - SUPPORTED_METHODS.contains(inputMessage.getMethod())) { - arg = readWithMessageConverters(inputMessage, methodParam, paramType); - if (arg == null) { - if (methodParam.getParameterAnnotation(RequestBody.class).required()) { - throw new HttpMessageNotReadableException("Required request body is missing: " + - methodParam.getMethod().toGenericString()); - } + arg = readWithMessageConverters(inputMessage, methodParam, paramType); + if (arg == null) { + if (methodParam.getParameterAnnotation(RequestBody.class).required()) { + throw new HttpMessageNotReadableException("Required request body is missing: " + + methodParam.getMethod().toGenericString()); } } return arg; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java index 30b54e06ad..44d3e400a9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java @@ -16,15 +16,10 @@ package org.springframework.web.servlet.mvc.method.annotation; -import static org.junit.Assert.*; -import static org.mockito.BDDMockito.*; -import static org.springframework.web.servlet.HandlerMapping.*; - import java.lang.reflect.Method; import java.net.URI; import java.nio.charset.Charset; import java.text.SimpleDateFormat; -import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.Locale; @@ -53,6 +48,10 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.method.support.ModelAndViewContainer; +import static org.junit.Assert.*; +import static org.mockito.BDDMockito.*; +import static org.springframework.web.servlet.HandlerMapping.*; + /** * Test fixture for {@link HttpEntityMethodProcessor} delegating to a mock * {@link HttpMessageConverter}. @@ -113,7 +112,7 @@ public class HttpEntityMethodProcessorMockTests { returnTypeInt = new MethodParameter(getClass().getMethod("handle3"), -1); mavContainer = new ModelAndViewContainer(); - servletRequest = new MockHttpServletRequest("GET", "/foo"); + servletRequest = new MockHttpServletRequest("POST", "/foo"); servletResponse = new MockHttpServletResponse(); webRequest = new ServletWebRequest(servletRequest, servletResponse); } @@ -185,7 +184,7 @@ public class HttpEntityMethodProcessorMockTests { MediaType contentType = MediaType.TEXT_PLAIN; servletRequest.addHeader("Content-Type", contentType.toString()); - given(messageConverter.getSupportedMediaTypes()).willReturn(Arrays.asList(contentType)); + given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(contentType)); given(messageConverter.canRead(String.class, contentType)).willReturn(false); processor.resolveArgument(paramHttpEntity, mavContainer, webRequest, null); @@ -266,7 +265,7 @@ public class HttpEntityMethodProcessorMockTests { servletRequest.addHeader("Accept", accepted.toString()); given(messageConverter.canWrite(String.class, null)).willReturn(true); - given(messageConverter.getSupportedMediaTypes()).willReturn(Arrays.asList(MediaType.TEXT_PLAIN)); + given(messageConverter.getSupportedMediaTypes()).willReturn(Collections.singletonList(MediaType.TEXT_PLAIN)); given(messageConverter.canWrite(String.class, accepted)).willReturn(false); processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java index e69889fb2b..812592dea4 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorTests.java @@ -16,16 +16,15 @@ package org.springframework.web.servlet.mvc.method.annotation; -import com.fasterxml.jackson.annotation.JsonTypeInfo; -import com.fasterxml.jackson.annotation.JsonTypeName; -import static org.junit.Assert.*; - import java.io.Serializable; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.JsonTypeName; import org.junit.Before; import org.junit.Test; @@ -46,6 +45,8 @@ import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.method.HandlerMethod; import org.springframework.web.method.support.ModelAndViewContainer; +import static org.junit.Assert.*; + /** * Test fixture with {@link HttpEntityMethodProcessor} delegating to * actual {@link HttpMessageConverter} instances. @@ -54,6 +55,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; * * @author Rossen Stoyanchev */ +@SuppressWarnings("unused") public class HttpEntityMethodProcessorTests { private MethodParameter paramList; @@ -81,7 +83,9 @@ public class HttpEntityMethodProcessorTests { binderFactory = new ValidatingBinderFactory(); servletRequest = new MockHttpServletRequest(); servletResponse = new MockHttpServletResponse(); + servletRequest.setMethod("POST"); webRequest = new ServletWebRequest(servletRequest, servletResponse); + } @Test @@ -109,7 +113,7 @@ public class HttpEntityMethodProcessorTests { this.servletRequest.setContent(new byte[0]); this.servletRequest.setContentType("application/json"); - List> converters = Arrays.asList(new MappingJackson2HttpMessageConverter()); + List> converters = Collections.singletonList(new MappingJackson2HttpMessageConverter()); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters); HttpEntity result = (HttpEntity) processor.resolveArgument(this.paramSimpleBean, @@ -196,9 +200,9 @@ public class HttpEntityMethodProcessorTests { private interface Identifiable extends Serializable { - public Long getId(); + Long getId(); - public void setId(Long id); + void setId(Long id); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java index 24b703572e..e345bda5d1 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java @@ -80,7 +80,12 @@ import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.util.UriComponentsBuilder; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; /** * A test fixture with a controller with all supported method signature styles @@ -102,6 +107,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { private MockHttpServletResponse response; + @Before public void setup() throws Exception { ConfigurableWebBindingInitializer bindingInitializer = new ConfigurableWebBindingInitializer(); @@ -123,6 +129,8 @@ public class RequestMappingHandlerAdapterIntegrationTests { request = new MockHttpServletRequest(); response = new MockHttpServletResponse(); + request.setMethod("POST"); + // Expose request to the current thread (for SpEL expressions) RequestContextHolder.setRequestAttributes(new ServletWebRequest(request)); } @@ -132,6 +140,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { RequestContextHolder.resetRequestAttributes(); } + @Test public void handle() throws Exception { -- GitLab