Fix issue with RequestBody(required=true)

Issue: SPR-11828
spring-projects · Jun 6, 2014 · de1a41a · de1a41a
1 parent c269d27
commit de1a41a
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 35 deletions.
diff --git a/...springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java b/...springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -153,41 +153,45 @@ protected <T> Object readWithMessageConverters(NativeWebRequest webRequest,
 		final HttpServletRequest servletRequest = webRequest.getNativeRequest(HttpServletRequest.class);
 		HttpInputMessage inputMessage = new ServletServerHttpRequest(servletRequest);
 
-		RequestBody annot = methodParam.getParameterAnnotation(RequestBody.class);
-		if (!annot.required()) {
-			InputStream inputStream = inputMessage.getBody();
-			if (inputStream == null) {
-				return null;
+		InputStream inputStream = inputMessage.getBody();
+		if (inputStream == null) {
+			return handleEmptyBody(methodParam);
+		}
+		else if (inputStream.markSupported()) {
+			inputStream.mark(1);
+			if (inputStream.read() == -1) {
+				return handleEmptyBody(methodParam);
 			}
-			else if (inputStream.markSupported()) {
-				inputStream.mark(1);
-				if (inputStream.read() == -1) {
-					return null;
-				}
-				inputStream.reset();
+			inputStream.reset();
+		}
+		else {
+			final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream);
+			int b = pushbackInputStream.read();
+			if (b == -1) {
+				return handleEmptyBody(methodParam);
 			}
 			else {
-				final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream);
-				int b = pushbackInputStream.read();
-				if (b == -1) {
-					return null;
-				}
-				else {
-					pushbackInputStream.unread(b);
-				}
-				inputMessage = new ServletServerHttpRequest(servletRequest) {
-					@Override
-					public InputStream getBody() throws IOException {
-						// Form POST should not get here
-						return pushbackInputStream;
-					}
-				};
+				pushbackInputStream.unread(b);
 			}
+			inputMessage = new ServletServerHttpRequest(servletRequest) {
+				@Override
+				public InputStream getBody() throws IOException {
+					// Form POST should not get here
+					return pushbackInputStream;
+				}
+			};
 		}
 
 		return super.readWithMessageConverters(inputMessage, methodParam, paramType);
 	}
 
+	private Object handleEmptyBody(MethodParameter param) {
+		if (param.getParameterAnnotation(RequestBody.class).required()) {
+			throw new HttpMessageNotReadableException("Required request body content is missing: " + param);
+		}
+		return null;
+	}
+
 	@Override
 	public void handleReturnValue(Object returnValue, MethodParameter returnType,
 			ModelAndViewContainer mavContainer, NativeWebRequest webRequest)

diff --git a/...mework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java b/...mework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.lang.reflect.Method;
+import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -32,6 +33,7 @@
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.HttpMessageConverter;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.mock.web.test.MockHttpServletResponse;
 import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
@@ -123,7 +125,7 @@ public void resolveArgument() throws Exception {
 		servletRequest.addHeader("Content-Type", contentType.toString());
 
 		String body = "Foo";
-		servletRequest.setContent(body.getBytes());
+		servletRequest.setContent(body.getBytes(Charset.forName("UTF-8")));
 
 		given(messageConverter.canRead(String.class, contentType)).willReturn(true);
 		given(messageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(body);
@@ -139,7 +141,8 @@ public void resolveArgumentNotValid() throws Exception {
 		try {
 			testResolveArgumentWithValidation(new SimpleBean(null));
 			fail("Expected exception");
-		} catch (MethodArgumentNotValidException e) {
+		}
+		catch (MethodArgumentNotValidException e) {
 			assertEquals("simpleBean", e.getBindingResult().getObjectName());
 			assertEquals(1, e.getBindingResult().getErrorCount());
 			assertNotNull(e.getBindingResult().getFieldError("name"));
@@ -154,7 +157,7 @@ public void resolveArgumentValid() throws Exception {
 	private void testResolveArgumentWithValidation(SimpleBean simpleBean) throws IOException, Exception {
 		MediaType contentType = MediaType.TEXT_PLAIN;
 		servletRequest.addHeader("Content-Type", contentType.toString());
-		servletRequest.setContent(new byte[] {});
+		servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
 
 		@SuppressWarnings("unchecked")
 		HttpMessageConverter<SimpleBean> beanConverter = mock(HttpMessageConverter.class);
@@ -170,6 +173,7 @@ private void testResolveArgumentWithValidation(SimpleBean simpleBean) throws IOE
 	public void resolveArgumentCannotRead() throws Exception {
 		MediaType contentType = MediaType.TEXT_PLAIN;
 		servletRequest.addHeader("Content-Type", contentType.toString());
+		servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
 
 		given(messageConverter.canRead(String.class, contentType)).willReturn(false);
 
@@ -178,6 +182,7 @@ public void resolveArgumentCannotRead() throws Exception {
 
 	@Test
 	public void resolveArgumentNoContentType() throws Exception {
+		servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
 		given(messageConverter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).willReturn(false);
 		try {
 			processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
@@ -190,14 +195,23 @@ public void resolveArgumentNoContentType() throws Exception {
 	@Test(expected = HttpMediaTypeNotSupportedException.class)
 	public void resolveArgumentInvalidContentType() throws Exception {
 		this.servletRequest.setContentType("bad");
+		servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
 		processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
 	}
 
+	// SPR-9942
+
+	@Test(expected = HttpMessageNotReadableException.class)
+	public void resolveArgumentRequiredNoContent() throws Exception {
+		servletRequest.setContentType(MediaType.TEXT_PLAIN_VALUE);
+		servletRequest.setContent(new byte[0]);
+		given(messageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
+		given(messageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(null);
+		assertNull(processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, new ValidatingBinderFactory()));
+	}
+
 	@Test
 	public void resolveArgumentNotRequiredNoContent() throws Exception {
-		servletRequest.setContent(null);
-		assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
-
 		servletRequest.setContent(new byte[0]);
 		assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
 	}

diff --git a/...gframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/...gframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -34,6 +34,7 @@
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.ByteArrayHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConverter;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.http.converter.StringHttpMessageConverter;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
 import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter;
@@ -174,6 +175,18 @@ public void resolveArgumentClassString() throws Exception {
 		assertEquals("foobarbaz", result);
 	}
 
+	// SPR-9942
+
+	@Test(expected = HttpMessageNotReadableException.class)
+	public void resolveArgumentRequiredNoContent() throws Exception {
+		this.servletRequest.setContent(new byte[0]);
+		this.servletRequest.setContentType("text/plain");
+		List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
+		converters.add(new StringHttpMessageConverter());
+		RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
+		processor.resolveArgument(paramString, mavContainer, webRequest, binderFactory);
+	}
+
 	// SPR-9964
 
 	@Test