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

Author: rstoyanchev

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java

@@ -22,6 +22,7 @@
 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.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 @@
  */
 public abstract class AbstractMessageConverterMethodArgumentResolver implements HandlerMethodArgumentResolver {
 
+	private static final List<HttpMethod> SUPPORTED_METHODS =
+			Arrays.asList(HttpMethod.POST, HttpMethod.PUT, HttpMethod.PATCH);
+
 	private static final Object NO_VALUE = new Object();
 
 
@@ -170,6 +176,7 @@ protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage,
 			targetClass = (Class<T>) resolvableType.resolve();
 		}
 
+		HttpMethod httpMethod = ((HttpRequest) inputMessage).getMethod();
 		inputMessage = new EmptyBodyCheckingHttpInputMessage(inputMessage);
 		Object body = NO_VALUE;
 
@@ -213,6 +220,9 @@ else if (targetClass != null) {
 		}
 
 		if (body == NO_VALUE) {
+			if (!SUPPORTED_METHODS.contains(httpMethod)) {
+				return null;
+			}
 			throw new HttpMediaTypeNotSupportedException(contentType, this.allSupportedMediaTypes);
 		}
 
@@ -273,6 +283,8 @@ private static class EmptyBodyCheckingHttpInputMessage implements HttpInputMessa
 
 		private final InputStream body;
 
+		private final HttpMethod method;
+
 
 		public EmptyBodyCheckingHttpInputMessage(HttpInputMessage inputMessage) throws IOException {
 			this.headers = inputMessage.getHeaders();
@@ -296,6 +308,7 @@ else if (inputStream.markSupported()) {
 					pushbackInputStream.unread(b);
 				}
 			}
+			this.method = ((HttpRequest) inputMessage).getMethod();
 		}
 
 		@Override
@@ -307,6 +320,10 @@ public HttpHeaders getHeaders() {
 		public InputStream getBody() throws IOException {
 			return this.body;
 		}
+
+		public HttpMethod getMethod() {
+			return this.method;
+		}
 	}
 
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java

@@ -150,14 +150,11 @@ protected <T> Object readWithMessageConverters(NativeWebRequest webRequest, Meth
 		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;

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.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 void setUp() throws Exception {
 		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 void resolveArgumentNotReadable() throws Exception {
 		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 void handleReturnValueNotAcceptable() throws Exception {
 		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);

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.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 @@
  *
  * @author Rossen Stoyanchev
  */
+@SuppressWarnings("unused")
 public class HttpEntityMethodProcessorTests {
 
 	private MethodParameter paramList;
@@ -81,7 +83,9 @@ public void setUp() throws Exception {
 		binderFactory = new ValidatingBinderFactory();
 		servletRequest = new MockHttpServletRequest();
 		servletResponse = new MockHttpServletResponse();
+		servletRequest.setMethod("POST");
 		webRequest = new ServletWebRequest(servletRequest, servletResponse);
+
 	}
 
 	@Test
@@ -109,7 +113,7 @@ public void resolveArgumentWithEmptyBody() throws Exception {
 		this.servletRequest.setContent(new byte[0]);
 		this.servletRequest.setContentType("application/json");
 
-		List<HttpMessageConverter<?>> converters = Arrays.asList(new MappingJackson2HttpMessageConverter());
+		List<HttpMessageConverter<?>> converters = Collections.singletonList(new MappingJackson2HttpMessageConverter());
 		HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters);
 
 		HttpEntity<?> result = (HttpEntity<?>) processor.resolveArgument(this.paramSimpleBean,
@@ -196,9 +200,9 @@ private static class MySimpleParameterizedController extends MyParameterizedCont
 
 	private interface Identifiable extends Serializable {
 
-		public Long getId();
+		Long getId();
 
-		public void setId(Long id);
+		void setId(Long id);
 	}
 
 

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java

@@ -80,7 +80,12 @@
 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 void setup() throws Exception {
 		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 void teardown() {
 		RequestContextHolder.resetRequestAttributes();
 	}
 
+
 	@Test
 	public void handle() throws Exception {