Skip to content

Commit de1a41a

Browse files
committed
Fix issue with RequestBody(required=true)
Issue: SPR-11828
1 parent c269d27 commit de1a41a

File tree

3 files changed

+66
-35
lines changed

3 files changed

+66
-35
lines changed

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

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -153,41 +153,45 @@ protected <T> Object readWithMessageConverters(NativeWebRequest webRequest,
153153
final HttpServletRequest servletRequest = webRequest.getNativeRequest(HttpServletRequest.class);
154154
HttpInputMessage inputMessage = new ServletServerHttpRequest(servletRequest);
155155

156-
RequestBody annot = methodParam.getParameterAnnotation(RequestBody.class);
157-
if (!annot.required()) {
158-
InputStream inputStream = inputMessage.getBody();
159-
if (inputStream == null) {
160-
return null;
156+
InputStream inputStream = inputMessage.getBody();
157+
if (inputStream == null) {
158+
return handleEmptyBody(methodParam);
159+
}
160+
else if (inputStream.markSupported()) {
161+
inputStream.mark(1);
162+
if (inputStream.read() == -1) {
163+
return handleEmptyBody(methodParam);
161164
}
162-
else if (inputStream.markSupported()) {
163-
inputStream.mark(1);
164-
if (inputStream.read() == -1) {
165-
return null;
166-
}
167-
inputStream.reset();
165+
inputStream.reset();
166+
}
167+
else {
168+
final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream);
169+
int b = pushbackInputStream.read();
170+
if (b == -1) {
171+
return handleEmptyBody(methodParam);
168172
}
169173
else {
170-
final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream);
171-
int b = pushbackInputStream.read();
172-
if (b == -1) {
173-
return null;
174-
}
175-
else {
176-
pushbackInputStream.unread(b);
177-
}
178-
inputMessage = new ServletServerHttpRequest(servletRequest) {
179-
@Override
180-
public InputStream getBody() throws IOException {
181-
// Form POST should not get here
182-
return pushbackInputStream;
183-
}
184-
};
174+
pushbackInputStream.unread(b);
185175
}
176+
inputMessage = new ServletServerHttpRequest(servletRequest) {
177+
@Override
178+
public InputStream getBody() throws IOException {
179+
// Form POST should not get here
180+
return pushbackInputStream;
181+
}
182+
};
186183
}
187184

188185
return super.readWithMessageConverters(inputMessage, methodParam, paramType);
189186
}
190187

188+
private Object handleEmptyBody(MethodParameter param) {
189+
if (param.getParameterAnnotation(RequestBody.class).required()) {
190+
throw new HttpMessageNotReadableException("Required request body content is missing: " + param);
191+
}
192+
return null;
193+
}
194+
191195
@Override
192196
public void handleReturnValue(Object returnValue, MethodParameter returnType,
193197
ModelAndViewContainer mavContainer, NativeWebRequest webRequest)

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818

1919
import java.io.IOException;
2020
import java.lang.reflect.Method;
21+
import java.nio.charset.Charset;
2122
import java.util.Arrays;
2223
import java.util.Collections;
2324
import java.util.List;
@@ -32,6 +33,7 @@
3233
import org.springframework.http.HttpOutputMessage;
3334
import org.springframework.http.MediaType;
3435
import org.springframework.http.converter.HttpMessageConverter;
36+
import org.springframework.http.converter.HttpMessageNotReadableException;
3537
import org.springframework.mock.web.test.MockHttpServletRequest;
3638
import org.springframework.mock.web.test.MockHttpServletResponse;
3739
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
@@ -123,7 +125,7 @@ public void resolveArgument() throws Exception {
123125
servletRequest.addHeader("Content-Type", contentType.toString());
124126

125127
String body = "Foo";
126-
servletRequest.setContent(body.getBytes());
128+
servletRequest.setContent(body.getBytes(Charset.forName("UTF-8")));
127129

128130
given(messageConverter.canRead(String.class, contentType)).willReturn(true);
129131
given(messageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(body);
@@ -139,7 +141,8 @@ public void resolveArgumentNotValid() throws Exception {
139141
try {
140142
testResolveArgumentWithValidation(new SimpleBean(null));
141143
fail("Expected exception");
142-
} catch (MethodArgumentNotValidException e) {
144+
}
145+
catch (MethodArgumentNotValidException e) {
143146
assertEquals("simpleBean", e.getBindingResult().getObjectName());
144147
assertEquals(1, e.getBindingResult().getErrorCount());
145148
assertNotNull(e.getBindingResult().getFieldError("name"));
@@ -154,7 +157,7 @@ public void resolveArgumentValid() throws Exception {
154157
private void testResolveArgumentWithValidation(SimpleBean simpleBean) throws IOException, Exception {
155158
MediaType contentType = MediaType.TEXT_PLAIN;
156159
servletRequest.addHeader("Content-Type", contentType.toString());
157-
servletRequest.setContent(new byte[] {});
160+
servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
158161

159162
@SuppressWarnings("unchecked")
160163
HttpMessageConverter<SimpleBean> beanConverter = mock(HttpMessageConverter.class);
@@ -170,6 +173,7 @@ private void testResolveArgumentWithValidation(SimpleBean simpleBean) throws IOE
170173
public void resolveArgumentCannotRead() throws Exception {
171174
MediaType contentType = MediaType.TEXT_PLAIN;
172175
servletRequest.addHeader("Content-Type", contentType.toString());
176+
servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
173177

174178
given(messageConverter.canRead(String.class, contentType)).willReturn(false);
175179

@@ -178,6 +182,7 @@ public void resolveArgumentCannotRead() throws Exception {
178182

179183
@Test
180184
public void resolveArgumentNoContentType() throws Exception {
185+
servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
181186
given(messageConverter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).willReturn(false);
182187
try {
183188
processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
@@ -190,14 +195,23 @@ public void resolveArgumentNoContentType() throws Exception {
190195
@Test(expected = HttpMediaTypeNotSupportedException.class)
191196
public void resolveArgumentInvalidContentType() throws Exception {
192197
this.servletRequest.setContentType("bad");
198+
servletRequest.setContent("payload".getBytes(Charset.forName("UTF-8")));
193199
processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
194200
}
195201

202+
// SPR-9942
203+
204+
@Test(expected = HttpMessageNotReadableException.class)
205+
public void resolveArgumentRequiredNoContent() throws Exception {
206+
servletRequest.setContentType(MediaType.TEXT_PLAIN_VALUE);
207+
servletRequest.setContent(new byte[0]);
208+
given(messageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true);
209+
given(messageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(null);
210+
assertNull(processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, new ValidatingBinderFactory()));
211+
}
212+
196213
@Test
197214
public void resolveArgumentNotRequiredNoContent() throws Exception {
198-
servletRequest.setContent(null);
199-
assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
200-
201215
servletRequest.setContent(new byte[0]);
202216
assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
203217
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
3434
import org.springframework.http.ResponseEntity;
3535
import org.springframework.http.converter.ByteArrayHttpMessageConverter;
3636
import org.springframework.http.converter.HttpMessageConverter;
37+
import org.springframework.http.converter.HttpMessageNotReadableException;
3738
import org.springframework.http.converter.StringHttpMessageConverter;
3839
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
3940
import org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter;
@@ -174,6 +175,18 @@ public void resolveArgumentClassString() throws Exception {
174175
assertEquals("foobarbaz", result);
175176
}
176177

178+
// SPR-9942
179+
180+
@Test(expected = HttpMessageNotReadableException.class)
181+
public void resolveArgumentRequiredNoContent() throws Exception {
182+
this.servletRequest.setContent(new byte[0]);
183+
this.servletRequest.setContentType("text/plain");
184+
List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
185+
converters.add(new StringHttpMessageConverter());
186+
RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
187+
processor.resolveArgument(paramString, mavContainer, webRequest, binderFactory);
188+
}
189+
177190
// SPR-9964
178191

179192
@Test

0 commit comments

Comments
 (0)