From de1a41ac275da81ead9dfd241c6ca1266adab75a Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 6 Jun 2014 12:07:14 -0400 Subject: [PATCH] Fix issue with RequestBody(required=true) Issue: SPR-11828 --- .../RequestResponseBodyMethodProcessor.java | 58 ++++++++++--------- ...tResponseBodyMethodProcessorMockTests.java | 28 ++++++--- ...questResponseBodyMethodProcessorTests.java | 15 ++++- 3 files changed, 66 insertions(+), 35 deletions(-) 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 c084ef64a5..7001e7a619 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 @@ -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 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter 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/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java index a4abfb0107..9411ec146d 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/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 @@ package org.springframework.web.servlet.mvc.method.annotation; 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.HttpInputMessage; 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 class RequestResponseBodyMethodProcessorMockTests { 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 class RequestResponseBodyMethodProcessorMockTests { 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 class RequestResponseBodyMethodProcessorMockTests { 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 beanConverter = mock(HttpMessageConverter.class); @@ -170,6 +173,7 @@ public class RequestResponseBodyMethodProcessorMockTests { 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 class RequestResponseBodyMethodProcessorMockTests { @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 class RequestResponseBodyMethodProcessorMockTests { @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/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java index 08dcd9bb78..146c3f1afc 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/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.MediaType; 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 class RequestResponseBodyMethodProcessorTests { 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> converters = new ArrayList>(); + converters.add(new StringHttpMessageConverter()); + RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters); + processor.resolveArgument(paramString, mavContainer, webRequest, binderFactory); + } + // SPR-9964 @Test