Skip to content

Commit c0baea5

Browse files
rstoyanchevrstoyanchev
committed
Fix issue with generic @RequestBody arguments
The original commit c9b7b1 ensured the ability to read parameterized type @RequestBody arguments via GenericHttpMessageConverter (e.g. application/json and List<String>). However, it also affected the ability to read @RequestBody arguments that happen are parameterized but aren't treated as such (e.g. application/x-www-form-urlencoded and MultiValueMap<String, String>). This commit corrects the issue. Issue: SPR-9570
1 parent 0e3aa0e commit c0baea5

File tree

4 files changed

+101
-36
lines changed

4 files changed

+101
-36
lines changed

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

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

1919
import java.io.IOException;
20+
import java.lang.reflect.Array;
21+
import java.lang.reflect.GenericArrayType;
22+
import java.lang.reflect.ParameterizedType;
2023
import java.lang.reflect.Type;
2124
import java.util.ArrayList;
2225
import java.util.Collections;
@@ -107,15 +110,15 @@ protected <T> Object readWithMessageConverters(NativeWebRequest webRequest,
107110
* @throws HttpMediaTypeNotSupportedException if no suitable message converter is found
108111
*/
109112
@SuppressWarnings("unchecked")
110-
protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage, MethodParameter methodParam,
111-
Type paramType) throws IOException, HttpMediaTypeNotSupportedException {
113+
protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage,
114+
MethodParameter methodParam, Type paramType) throws IOException, HttpMediaTypeNotSupportedException {
112115

113116
MediaType contentType = inputMessage.getHeaders().getContentType();
114117
if (contentType == null) {
115118
contentType = MediaType.APPLICATION_OCTET_STREAM;
116119
}
117120

118-
Class<T> paramClass = (paramType instanceof Class) ? (Class) paramType : null;
121+
Class<T> paramClass = getParamClass(paramType);
119122

120123
for (HttpMessageConverter<?> messageConverter : this.messageConverters) {
121124
if (messageConverter instanceof GenericHttpMessageConverter) {
@@ -142,6 +145,26 @@ protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage, Me
142145
throw new HttpMediaTypeNotSupportedException(contentType, allSupportedMediaTypes);
143146
}
144147

148+
private Class getParamClass(Type paramType) {
149+
if (paramType instanceof Class) {
150+
return (Class) paramType;
151+
}
152+
else if (paramType instanceof GenericArrayType) {
153+
Type componentType = ((GenericArrayType) paramType).getGenericComponentType();
154+
if (componentType instanceof Class) {
155+
// Surely, there should be a nicer way to determine the array type
156+
return Array.newInstance((Class<?>) componentType, 0).getClass();
157+
}
158+
}
159+
else if (paramType instanceof ParameterizedType) {
160+
ParameterizedType parameterizedType = (ParameterizedType) paramType;
161+
if (parameterizedType.getRawType() instanceof Class) {
162+
return (Class) parameterizedType.getRawType();
163+
}
164+
}
165+
return null;
166+
}
167+
145168
/**
146169
* Creates a new {@link HttpInputMessage} from the given {@link NativeWebRequest}.
147170
*

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

Lines changed: 4 additions & 18 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.lang.reflect.Array;
21-
import java.lang.reflect.GenericArrayType;
2220
import java.lang.reflect.ParameterizedType;
2321
import java.lang.reflect.Type;
2422
import java.util.List;
@@ -89,23 +87,11 @@ private Type getHttpEntityType(MethodParameter parameter) {
8987
Assert.isAssignable(HttpEntity.class, parameter.getParameterType());
9088
ParameterizedType type = (ParameterizedType) parameter.getGenericParameterType();
9189
if (type.getActualTypeArguments().length == 1) {
92-
Type typeArgument = type.getActualTypeArguments()[0];
93-
if (typeArgument instanceof Class) {
94-
return (Class<?>) typeArgument;
95-
}
96-
else if (typeArgument instanceof GenericArrayType) {
97-
Type componentType = ((GenericArrayType) typeArgument).getGenericComponentType();
98-
if (componentType instanceof Class) {
99-
// Surely, there should be a nicer way to determine the array type
100-
return Array.newInstance((Class<?>) componentType, 0).getClass();
101-
}
102-
}
103-
else if (typeArgument instanceof ParameterizedType) {
104-
return typeArgument;
105-
}
90+
return type.getActualTypeArguments()[0];
10691
}
107-
throw new IllegalArgumentException("HttpEntity parameter (" + parameter.getParameterName() + ") "
108-
+ "in method " + parameter.getMethod() + "is not parameterized");
92+
throw new IllegalArgumentException("HttpEntity parameter ("
93+
+ parameter.getParameterName() + ") in method " + parameter.getMethod()
94+
+ " is not parameterized or has more than one parameter");
10995
}
11096

11197
public void handleReturnValue(

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -190,30 +190,35 @@ private void testResolveArgumentWithValidation(SimpleBean simpleBean) throws IOE
190190
}
191191

192192
@Test(expected = HttpMediaTypeNotSupportedException.class)
193-
public void resolveArgumentNotReadable() throws Exception {
193+
public void resolveArgumentCannotRead() throws Exception {
194194
MediaType contentType = MediaType.TEXT_PLAIN;
195195
servletRequest.addHeader("Content-Type", contentType.toString());
196-
servletRequest.setContent(new byte[] {});
197196

198197
expect(messageConverter.canRead(String.class, contentType)).andReturn(false);
199198
replay(messageConverter);
200199

201200
processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
202201
}
203202

204-
@Test(expected = HttpMediaTypeNotSupportedException.class)
203+
@Test
205204
public void resolveArgumentNoContentType() throws Exception {
206-
servletRequest.setContent(new byte[] {});
207-
processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
208-
}
209-
210-
@Test(expected = HttpMessageNotReadableException.class)
211-
public void resolveArgumentRequiredNoContent() throws Exception {
212-
processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
205+
expect(messageConverter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).andReturn(false);
206+
replay(messageConverter);
207+
try {
208+
processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
209+
fail("Expected exception");
210+
}
211+
catch (HttpMediaTypeNotSupportedException ex) {
212+
}
213+
verify(messageConverter);
213214
}
214215

215216
@Test
216217
public void resolveArgumentNotRequiredNoContent() throws Exception {
218+
servletRequest.setContent(null);
219+
assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
220+
221+
servletRequest.setContent(new byte[0]);
217222
assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
218223
}
219224

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

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,21 @@
2121

2222
import java.lang.reflect.Method;
2323
import java.util.ArrayList;
24+
import java.util.Collection;
2425
import java.util.List;
2526

2627
import org.junit.Before;
2728
import org.junit.Test;
2829
import org.springframework.core.MethodParameter;
30+
import org.springframework.http.MediaType;
2931
import org.springframework.http.converter.ByteArrayHttpMessageConverter;
3032
import org.springframework.http.converter.HttpMessageConverter;
3133
import org.springframework.http.converter.StringHttpMessageConverter;
3234
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
35+
import org.springframework.http.converter.xml.XmlAwareFormHttpMessageConverter;
3336
import org.springframework.mock.web.MockHttpServletRequest;
3437
import org.springframework.mock.web.MockHttpServletResponse;
38+
import org.springframework.util.MultiValueMap;
3539
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
3640
import org.springframework.web.bind.WebDataBinder;
3741
import org.springframework.web.bind.annotation.RequestBody;
@@ -52,6 +56,8 @@ public class RequestResponseBodyMethodProcessorTests {
5256

5357
private MethodParameter paramGenericList;
5458
private MethodParameter paramSimpleBean;
59+
private MethodParameter paramMultiValueMap;
60+
private MethodParameter paramString;
5561
private MethodParameter returnTypeString;
5662

5763
private ModelAndViewContainer mavContainer;
@@ -65,9 +71,13 @@ public class RequestResponseBodyMethodProcessorTests {
6571
@Before
6672
public void setUp() throws Exception {
6773

68-
Method method = getClass().getMethod("handle", List.class, SimpleBean.class);
74+
Method method = getClass().getMethod("handle",
75+
List.class, SimpleBean.class, MultiValueMap.class, String.class);
76+
6977
paramGenericList = new MethodParameter(method, 0);
7078
paramSimpleBean = new MethodParameter(method, 1);
79+
paramMultiValueMap = new MethodParameter(method, 2);
80+
paramString = new MethodParameter(method, 3);
7181
returnTypeString = new MethodParameter(method, -1);
7282

7383
mavContainer = new ModelAndViewContainer();
@@ -79,10 +89,10 @@ public void setUp() throws Exception {
7989

8090

8191
@Test
82-
public void resolveGenericArgument() throws Exception {
92+
public void resolveArgumentParameterizedType() throws Exception {
8393
String content = "[{\"name\" : \"Jad\"}, {\"name\" : \"Robert\"}]";
8494
this.servletRequest.setContent(content.getBytes("UTF-8"));
85-
this.servletRequest.setContentType("application/json");
95+
this.servletRequest.setContentType(MediaType.APPLICATION_JSON_VALUE);
8696

8797
List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
8898
converters.add(new MappingJackson2HttpMessageConverter());
@@ -98,7 +108,26 @@ public void resolveGenericArgument() throws Exception {
98108
}
99109

100110
@Test
101-
public void resolveArgument() throws Exception {
111+
public void resolveArgumentRawTypeFromParameterizedType() throws Exception {
112+
String content = "fruit=apple&vegetable=kale";
113+
this.servletRequest.setContent(content.getBytes("UTF-8"));
114+
this.servletRequest.setContentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE);
115+
116+
List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
117+
converters.add(new XmlAwareFormHttpMessageConverter());
118+
RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
119+
120+
@SuppressWarnings("unchecked")
121+
MultiValueMap<String, String> result = (MultiValueMap<String, String>) processor.resolveArgument(
122+
paramMultiValueMap, mavContainer, webRequest, new ValidatingBinderFactory());
123+
124+
assertNotNull(result);
125+
assertEquals("apple", result.getFirst("fruit"));
126+
assertEquals("kale", result.getFirst("vegetable"));
127+
}
128+
129+
@Test
130+
public void resolveArgumentClassJson() throws Exception {
102131
String content = "{\"name\" : \"Jad\"}";
103132
this.servletRequest.setContent(content.getBytes("UTF-8"));
104133
this.servletRequest.setContentType("application/json");
@@ -114,6 +143,23 @@ public void resolveArgument() throws Exception {
114143
assertEquals("Jad", result.getName());
115144
}
116145

146+
@Test
147+
public void resolveArgumentClassString() throws Exception {
148+
String content = "foobarbaz";
149+
this.servletRequest.setContent(content.getBytes("UTF-8"));
150+
this.servletRequest.setContentType("application/json");
151+
152+
List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
153+
converters.add(new StringHttpMessageConverter());
154+
RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
155+
156+
String result = (String) processor.resolveArgument(
157+
paramString, mavContainer, webRequest, new ValidatingBinderFactory());
158+
159+
assertNotNull(result);
160+
assertEquals("foobarbaz", result);
161+
}
162+
117163
// SPR-9160
118164

119165
@Test
@@ -158,7 +204,12 @@ public void handleReturnValueStringAcceptCharset() throws Exception {
158204
}
159205

160206

161-
public String handle(@RequestBody List<SimpleBean> list, @RequestBody SimpleBean simpleBean) {
207+
public String handle(
208+
@RequestBody List<SimpleBean> list,
209+
@RequestBody SimpleBean simpleBean,
210+
@RequestBody MultiValueMap<String, String> multiValueMap,
211+
@RequestBody String string) {
212+
162213
return null;
163214
}
164215

0 commit comments

Comments
 (0)