Skip to content

Commit 36ed4df

Browse files
rstoyanchevrstoyanchev
committed
Improve empty request body handling
The check for an empty request body InputStream is now in the base class AbstractMessageConverterMethodArgumentResolver shared for all arguments that involve reading with an HttpMessageConverter -- @RequestBody, @RequestPart, and HttpEntity. When an empty body is detected any configured RequestBodyAdvice is given a chance to select a default value or leave it as null. Issue: SPR-12778, SPR-12860, SPR-12861
1 parent 0556ed4 commit 36ed4df

9 files changed

+256
-107
lines changed

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

Lines changed: 79 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

1919
import java.io.IOException;
20+
import java.io.InputStream;
21+
import java.io.PushbackInputStream;
2022
import java.lang.annotation.Annotation;
2123
import java.lang.reflect.Type;
2224
import java.util.ArrayList;
@@ -33,6 +35,7 @@
3335
import org.springframework.core.MethodParameter;
3436
import org.springframework.core.ResolvableType;
3537
import org.springframework.core.annotation.AnnotationUtils;
38+
import org.springframework.http.HttpHeaders;
3639
import org.springframework.http.HttpInputMessage;
3740
import org.springframework.http.InvalidMediaTypeException;
3841
import org.springframework.http.MediaType;
@@ -57,6 +60,9 @@
5760
*/
5861
public abstract class AbstractMessageConverterMethodArgumentResolver implements HandlerMethodArgumentResolver {
5962

63+
private static final Object NO_VALUE = new Object();
64+
65+
6066
protected final Log logger = LogFactory.getLog(getClass());
6167

6268
protected final List<HttpMessageConverter<?>> messageConverters;
@@ -135,9 +141,8 @@ protected <T> Object readWithMessageConverters(NativeWebRequest webRequest,
135141
* @param <T> the expected type of the argument value to be created
136142
* @param inputMessage the HTTP input message representing the current request
137143
* @param param the method parameter descriptor (may be {@code null})
138-
* @param targetType the type of object to create, not necessarily the same as
139-
* the method parameter type (e.g. for {@code HttpEntity<String>} method
140-
* parameter the target type is String)
144+
* @param targetType the target type, not necessarily the same as the method
145+
* parameter type, e.g. for {@code HttpEntity<String>}.
141146
* @return the created method argument value
142147
* @throws IOException if the reading from the request fails
143148
* @throws HttpMediaTypeNotSupportedException if no suitable message converter is found
@@ -165,6 +170,9 @@ protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage,
165170
targetClass = (Class<T>) resolvableType.resolve();
166171
}
167172

173+
inputMessage = new EmptyBodyCheckingHttpInputMessage(inputMessage);
174+
Object body = NO_VALUE;
175+
168176
for (HttpMessageConverter<?> converter : this.messageConverters) {
169177
Class<HttpMessageConverter<?>> converterType = (Class<HttpMessageConverter<?>>) converter.getClass();
170178
if (converter instanceof GenericHttpMessageConverter) {
@@ -173,24 +181,42 @@ protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage,
173181
if (logger.isDebugEnabled()) {
174182
logger.debug("Read [" + targetType + "] as \"" + contentType + "\" with [" + converter + "]");
175183
}
176-
inputMessage = getAdvice().beforeBodyRead(inputMessage, param, targetType, converterType);
177-
T body = (T) genericConverter.read(targetType, contextClass, inputMessage);
178-
return getAdvice().afterBodyRead(body, inputMessage, param, targetType, converterType);
184+
if (inputMessage.getBody() != null) {
185+
inputMessage = getAdvice().beforeBodyRead(inputMessage, param, targetType, converterType);
186+
body = genericConverter.read(targetType, contextClass, inputMessage);
187+
body = getAdvice().afterBodyRead(body, inputMessage, param, targetType, converterType);
188+
}
189+
else {
190+
body = null;
191+
body = getAdvice().handleEmptyBody(body, inputMessage, param, targetType, converterType);
192+
}
193+
break;
179194
}
180195
}
181196
else if (targetClass != null) {
182197
if (converter.canRead(targetClass, contentType)) {
183198
if (logger.isDebugEnabled()) {
184199
logger.debug("Read [" + targetType + "] as \"" + contentType + "\" with [" + converter + "]");
185200
}
186-
inputMessage = getAdvice().beforeBodyRead(inputMessage, param, targetType, converterType);
187-
T body = ((HttpMessageConverter<T>) converter).read(targetClass, inputMessage);
188-
return getAdvice().afterBodyRead(body, inputMessage, param, targetType, converterType);
201+
if (inputMessage.getBody() != null) {
202+
inputMessage = getAdvice().beforeBodyRead(inputMessage, param, targetType, converterType);
203+
body = ((HttpMessageConverter<T>) converter).read(targetClass, inputMessage);
204+
body = getAdvice().afterBodyRead(body, inputMessage, param, targetType, converterType);
205+
}
206+
else {
207+
body = null;
208+
body = getAdvice().handleEmptyBody(body, inputMessage, param, targetType, converterType);
209+
}
210+
break;
189211
}
190212
}
191213
}
192214

193-
throw new HttpMediaTypeNotSupportedException(contentType, this.allSupportedMediaTypes);
215+
if (body == NO_VALUE) {
216+
throw new HttpMediaTypeNotSupportedException(contentType, this.allSupportedMediaTypes);
217+
}
218+
219+
return body;
194220
}
195221

196222
/**
@@ -240,4 +266,47 @@ protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter
240266
return !hasBindingResult;
241267
}
242268

269+
270+
private static class EmptyBodyCheckingHttpInputMessage implements HttpInputMessage {
271+
272+
private final HttpHeaders headers;
273+
274+
private final InputStream body;
275+
276+
277+
public EmptyBodyCheckingHttpInputMessage(HttpInputMessage inputMessage) throws IOException {
278+
this.headers = inputMessage.getHeaders();
279+
InputStream inputStream = inputMessage.getBody();
280+
if (inputStream == null) {
281+
this.body = null;
282+
}
283+
else if (inputStream.markSupported()) {
284+
inputStream.mark(1);
285+
this.body = (inputStream.read() != -1 ? inputStream : null);
286+
inputStream.reset();
287+
}
288+
else {
289+
PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream);
290+
int b = pushbackInputStream.read();
291+
if (b == -1) {
292+
this.body = null;
293+
}
294+
else {
295+
this.body = pushbackInputStream;
296+
pushbackInputStream.unread(b);
297+
}
298+
}
299+
}
300+
301+
@Override
302+
public HttpHeaders getHeaders() {
303+
return this.headers;
304+
}
305+
306+
@Override
307+
public InputStream getBody() throws IOException {
308+
return this.body;
309+
}
310+
}
311+
243312
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
import java.io.IOException;
2020
import java.lang.reflect.ParameterizedType;
2121
import java.lang.reflect.Type;
22+
import java.net.URI;
2223
import java.util.List;
2324

2425
import org.springframework.core.MethodParameter;
2526
import org.springframework.core.ResolvableType;
2627
import org.springframework.http.HttpEntity;
2728
import org.springframework.http.HttpHeaders;
29+
import org.springframework.http.HttpMethod;
2830
import org.springframework.http.RequestEntity;
2931
import org.springframework.http.ResponseEntity;
3032
import org.springframework.http.converter.HttpMessageConverter;
@@ -120,8 +122,9 @@ public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer m
120122

121123
Object body = readWithMessageConverters(webRequest, parameter, paramType);
122124
if (RequestEntity.class.equals(parameter.getParameterType())) {
123-
return new RequestEntity<Object>(body, inputMessage.getHeaders(),
124-
inputMessage.getMethod(), inputMessage.getURI());
125+
URI url = inputMessage.getURI();
126+
HttpMethod httpMethod = inputMessage.getMethod();
127+
return new RequestEntity<Object>(body, inputMessage.getHeaders(), httpMethod, url);
125128
}
126129
else {
127130
return new HttpEntity<Object>(body, inputMessage.getHeaders());

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

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

1919
import java.io.IOException;
20-
import java.io.InputStream;
21-
import java.io.PushbackInputStream;
2220
import java.lang.reflect.Type;
2321
import java.util.List;
2422

@@ -146,43 +144,14 @@ protected <T> Object readWithMessageConverters(NativeWebRequest webRequest, Meth
146144
HttpServletRequest servletRequest = webRequest.getNativeRequest(HttpServletRequest.class);
147145
HttpInputMessage inputMessage = new ServletServerHttpRequest(servletRequest);
148146

149-
InputStream inputStream = inputMessage.getBody();
150-
if (inputStream == null) {
151-
return handleEmptyBody(methodParam);
152-
}
153-
else if (inputStream.markSupported()) {
154-
inputStream.mark(1);
155-
if (inputStream.read() == -1) {
156-
return handleEmptyBody(methodParam);
157-
}
158-
inputStream.reset();
159-
}
160-
else {
161-
final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream);
162-
int b = pushbackInputStream.read();
163-
if (b == -1) {
164-
return handleEmptyBody(methodParam);
165-
}
166-
else {
167-
pushbackInputStream.unread(b);
147+
Object arg = readWithMessageConverters(inputMessage, methodParam, paramType);
148+
if (arg == null) {
149+
if (methodParam.getParameterAnnotation(RequestBody.class).required()) {
150+
throw new HttpMessageNotReadableException("Required request body is missing: " + methodParam);
168151
}
169-
inputMessage = new ServletServerHttpRequest(servletRequest) {
170-
@Override
171-
public InputStream getBody() {
172-
// Form POST should not get here
173-
return pushbackInputStream;
174-
}
175-
};
176152
}
177153

178-
return super.readWithMessageConverters(inputMessage, methodParam, paramType);
179-
}
180-
181-
private Object handleEmptyBody(MethodParameter param) {
182-
if (param.getParameterAnnotation(RequestBody.class).required()) {
183-
throw new HttpMessageNotReadableException("Required request body content is missing: " + param);
184-
}
185-
return null;
154+
return arg;
186155
}
187156

188157
@Override

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

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,19 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19+
import static org.junit.Assert.*;
20+
import static org.mockito.BDDMockito.any;
21+
import static org.mockito.BDDMockito.*;
22+
import static org.mockito.BDDMockito.isA;
23+
import static org.mockito.Matchers.eq;
24+
import static org.springframework.web.servlet.HandlerMapping.*;
25+
1926
import java.lang.reflect.Method;
2027
import java.net.URI;
21-
import java.text.SimpleDateFormat;
28+
import java.nio.charset.Charset;
2229
import java.util.Arrays;
2330
import java.util.Collections;
2431
import java.util.Date;
25-
import java.util.Locale;
26-
import java.util.TimeZone;
2732

2833
import org.junit.Before;
2934
import org.junit.Test;
@@ -48,13 +53,6 @@
4853
import org.springframework.web.context.request.ServletWebRequest;
4954
import org.springframework.web.method.support.ModelAndViewContainer;
5055

51-
import static org.junit.Assert.*;
52-
import static org.mockito.BDDMockito.any;
53-
import static org.mockito.BDDMockito.*;
54-
import static org.mockito.BDDMockito.isA;
55-
import static org.mockito.Matchers.eq;
56-
import static org.springframework.web.servlet.HandlerMapping.*;
57-
5856
/**
5957
* Test fixture for {@link HttpEntityMethodProcessor} delegating to a mock
6058
* {@link HttpMessageConverter}.
@@ -136,10 +134,12 @@ public void supportsReturnType() {
136134

137135
@Test
138136
public void resolveArgument() throws Exception {
137+
String body = "Foo";
138+
139139
MediaType contentType = MediaType.TEXT_PLAIN;
140140
servletRequest.addHeader("Content-Type", contentType.toString());
141+
servletRequest.setContent(body.getBytes(Charset.forName("UTF-8")));
141142

142-
String body = "Foo";
143143
given(messageConverter.canRead(String.class, contentType)).willReturn(true);
144144
given(messageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(body);
145145

@@ -152,14 +152,16 @@ public void resolveArgument() throws Exception {
152152

153153
@Test
154154
public void resolveArgumentRequestEntity() throws Exception {
155+
String body = "Foo";
156+
155157
MediaType contentType = MediaType.TEXT_PLAIN;
156158
servletRequest.addHeader("Content-Type", contentType.toString());
157159
servletRequest.setMethod("GET");
158160
servletRequest.setServerName("www.example.com");
159161
servletRequest.setServerPort(80);
160162
servletRequest.setRequestURI("/path");
163+
servletRequest.setContent(body.getBytes(Charset.forName("UTF-8")));
161164

162-
String body = "Foo";
163165
given(messageConverter.canRead(String.class, contentType)).willReturn(true);
164166
given(messageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn(body);
165167

@@ -226,6 +228,7 @@ public void handleReturnValueProduces() throws Exception {
226228
verify(messageConverter).write(eq(body), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class));
227229
}
228230

231+
@SuppressWarnings("unchecked")
229232
@Test
230233
public void handleReturnValueWithResponseBodyAdvice() throws Exception {
231234
ResponseEntity<String> returnValue = new ResponseEntity<>(HttpStatus.OK);
@@ -416,28 +419,35 @@ public void handleReturnTypeChangedETagAndLastModified() throws Exception {
416419
assertEquals(0, servletResponse.getContentAsByteArray().length);
417420
}
418421

419-
public ResponseEntity<String> handle1(HttpEntity<String> httpEntity, ResponseEntity<String> responseEntity, int i, RequestEntity<String> requestEntity) {
420-
return responseEntity;
422+
@SuppressWarnings("unused")
423+
public ResponseEntity<String> handle1(HttpEntity<String> httpEntity, ResponseEntity<String> entity,
424+
int i, RequestEntity<String> requestEntity) {
425+
426+
return entity;
421427
}
422428

429+
@SuppressWarnings("unused")
423430
public HttpEntity<?> handle2(HttpEntity<?> entity) {
424431
return entity;
425432
}
426433

434+
@SuppressWarnings("unused")
427435
public CustomHttpEntity handle2x(HttpEntity<?> entity) {
428436
return new CustomHttpEntity();
429437
}
430438

439+
@SuppressWarnings("unused")
431440
public int handle3() {
432441
return 42;
433442
}
434443

444+
@SuppressWarnings("unused")
435445
@RequestMapping(produces = {"text/html", "application/xhtml+xml"})
436446
public ResponseEntity<String> handle4() {
437447
return null;
438448
}
439449

440-
450+
@SuppressWarnings("unused")
441451
public static class CustomHttpEntity extends HttpEntity<Object> {
442452
}
443453

0 commit comments

Comments
 (0)