Skip to content

Commit a58eee6

Browse files
jhoellerjhoeller
committed
RequestParamMethodArgumentResolver defensively handles MethodParameter nesting level and java.util.Optional access
Issue: SPR-13850
1 parent 0866aa9 commit a58eee6

File tree

2 files changed

+67
-45
lines changed

2 files changed

+67
-45
lines changed

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2016 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.
@@ -20,7 +20,6 @@
2020
import java.util.Collection;
2121
import java.util.List;
2222
import java.util.Optional;
23-
2423
import javax.servlet.http.HttpServletRequest;
2524
import javax.servlet.http.Part;
2625

@@ -123,24 +122,23 @@ else if ("javax.servlet.http.Part".equals(parameter.getParameterType().getName()
123122
}
124123

125124
@Override
126-
@UsesJava8
127125
public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
128126
NativeWebRequest request, WebDataBinderFactory binderFactory) throws Exception {
129127

130128
HttpServletRequest servletRequest = request.getNativeRequest(HttpServletRequest.class);
131129
assertIsMultipartRequest(servletRequest);
132-
133130
MultipartHttpServletRequest multipartRequest =
134131
WebUtils.getNativeRequest(servletRequest, MultipartHttpServletRequest.class);
135132

133+
String partName = getPartName(parameter);
136134
Class<?> paramType = parameter.getParameterType();
137135
boolean optional = paramType.getName().equals("java.util.Optional");
138136
if (optional) {
137+
parameter = new MethodParameter(parameter);
139138
parameter.increaseNestingLevel();
140139
paramType = parameter.getNestedParameterType();
141140
}
142141

143-
String partName = getPartName(parameter);
144142
Object arg;
145143

146144
if (MultipartFile.class == paramType) {
@@ -194,7 +192,7 @@ else if (isPartArray(parameter)) {
194192
throw new MissingServletRequestPartException(partName);
195193
}
196194
if (optional) {
197-
arg = Optional.ofNullable(arg);
195+
arg = OptionalResolver.resolveValue(arg);
198196
}
199197

200198
return arg;
@@ -264,4 +262,16 @@ public static Object resolvePart(HttpServletRequest servletRequest) throws Excep
264262
}
265263
}
266264

265+
266+
/**
267+
* Inner class to avoid hard-coded dependency on Java 8 Optional type...
268+
*/
269+
@UsesJava8
270+
private static class OptionalResolver {
271+
272+
public static Object resolveValue(Object value) {
273+
return Optional.ofNullable(value);
274+
}
275+
}
276+
267277
}

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

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2016 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.
@@ -26,7 +26,6 @@
2626
import javax.validation.Valid;
2727
import javax.validation.constraints.NotNull;
2828

29-
import org.hamcrest.Matchers;
3029
import org.junit.Before;
3130
import org.junit.Test;
3231

@@ -157,14 +156,12 @@ public void supportsParameter() {
157156
@Test
158157
public void resolveMultipartFile() throws Exception {
159158
Object actual = resolver.resolveArgument(paramMultipartFile, null, webRequest, null);
160-
assertNotNull(actual);
161159
assertSame(multipartFile1, actual);
162160
}
163161

164162
@Test
165163
public void resolveMultipartFileList() throws Exception {
166164
Object actual = resolver.resolveArgument(paramMultipartFileList, null, webRequest, null);
167-
assertNotNull(actual);
168165
assertTrue(actual instanceof List);
169166
assertEquals(Arrays.asList(multipartFile1, multipartFile2), actual);
170167
}
@@ -175,6 +172,7 @@ public void resolveMultipartFileArray() throws Exception {
175172
assertNotNull(actual);
176173
assertTrue(actual instanceof MultipartFile[]);
177174
MultipartFile[] parts = (MultipartFile[]) actual;
175+
assertEquals(2, parts.length);
178176
assertEquals(parts[0], multipartFile1);
179177
assertEquals(parts[1], multipartFile2);
180178
}
@@ -202,7 +200,6 @@ public void resolvePartArgument() throws Exception {
202200
webRequest = new ServletWebRequest(request);
203201

204202
Object result = resolver.resolveArgument(paramPart, null, webRequest, null);
205-
206203
assertTrue(result instanceof Part);
207204
assertEquals("Invalid result", expected, result);
208205
}
@@ -219,7 +216,6 @@ public void resolvePartListArgument() throws Exception {
219216
webRequest = new ServletWebRequest(request);
220217

221218
Object result = resolver.resolveArgument(paramPartList, null, webRequest, null);
222-
223219
assertTrue(result instanceof List);
224220
assertEquals(Arrays.asList(part1, part2), result);
225221
}
@@ -236,10 +232,9 @@ public void resolvePartArrayArgument() throws Exception {
236232
webRequest = new ServletWebRequest(request);
237233

238234
Object result = resolver.resolveArgument(paramPartArray, null, webRequest, null);
239-
240235
assertTrue(result instanceof Part[]);
241236
Part[] parts = (Part[]) result;
242-
assertThat(parts, Matchers.arrayWithSize(2));
237+
assertEquals(2, parts.length);
243238
assertEquals(parts[0], part1);
244239
assertEquals(parts[1], part2);
245240
}
@@ -302,8 +297,8 @@ public void isMultipartRequest() throws Exception {
302297
@Test // SPR-9079
303298
public void isMultipartRequestPut() throws Exception {
304299
this.multipartRequest.setMethod("PUT");
305-
Object actual = resolver.resolveArgument(paramMultipartFile, null, webRequest, null);
306-
assertSame(multipartFile1, actual);
300+
Object actualValue = resolver.resolveArgument(paramMultipartFile, null, webRequest, null);
301+
assertSame(multipartFile1, actualValue);
307302
}
308303

309304
@Test
@@ -313,21 +308,25 @@ public void resolveOptionalMultipartFileArgument() throws Exception {
313308
request.addFile(expected);
314309
webRequest = new ServletWebRequest(request);
315310

316-
Object result = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
311+
Object actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
312+
assertTrue(actualValue instanceof Optional);
313+
assertEquals("Invalid result", expected, ((Optional) actualValue).get());
317314

318-
assertTrue(result instanceof Optional);
319-
assertEquals("Invalid result", expected, ((Optional) result).get());
315+
actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
316+
assertTrue(actualValue instanceof Optional);
317+
assertEquals("Invalid result", expected, ((Optional) actualValue).get());
320318
}
321319

322320
@Test
323321
public void resolveOptionalMultipartFileArgumentNotPresent() throws Exception {
324322
MockMultipartHttpServletRequest request = new MockMultipartHttpServletRequest();
325323
webRequest = new ServletWebRequest(request);
326324

327-
Object result = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
325+
Object actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
326+
assertEquals("Invalid argument value", Optional.empty(), actualValue);
328327

329-
assertTrue(result instanceof Optional);
330-
assertFalse("Invalid result", ((Optional) result).isPresent());
328+
actualValue = resolver.resolveArgument(optionalMultipartFile, null, webRequest, null);
329+
assertEquals("Invalid argument value", Optional.empty(), actualValue);
331330
}
332331

333332
@Test
@@ -339,10 +338,13 @@ public void resolveOptionalPartArgument() throws Exception {
339338
request.addPart(expected);
340339
webRequest = new ServletWebRequest(request);
341340

342-
Object result = resolver.resolveArgument(optionalPart, null, webRequest, null);
341+
Object actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null);
342+
assertTrue(actualValue instanceof Optional);
343+
assertEquals("Invalid result", expected, ((Optional) actualValue).get());
343344

344-
assertTrue(result instanceof Optional);
345-
assertEquals("Invalid result", expected, ((Optional) result).get());
345+
actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null);
346+
assertTrue(actualValue instanceof Optional);
347+
assertEquals("Invalid result", expected, ((Optional) actualValue).get());
346348
}
347349

348350
@Test
@@ -352,22 +354,26 @@ public void resolveOptionalPartArgumentNotPresent() throws Exception {
352354
request.setContentType("multipart/form-data");
353355
webRequest = new ServletWebRequest(request);
354356

355-
Object result = resolver.resolveArgument(optionalPart, null, webRequest, null);
357+
Object actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null);
358+
assertEquals("Invalid argument value", Optional.empty(), actualValue);
356359

357-
assertTrue(result instanceof Optional);
358-
assertFalse("Invalid result", ((Optional) result).isPresent());
360+
actualValue = resolver.resolveArgument(optionalPart, null, webRequest, null);
361+
assertEquals("Invalid argument value", Optional.empty(), actualValue);
359362
}
360363

361364
@Test
362365
public void resolveOptionalRequestPart() throws Exception {
363366
SimpleBean simpleBean = new SimpleBean("foo");
364-
365367
given(messageConverter.canRead(SimpleBean.class, MediaType.TEXT_PLAIN)).willReturn(true);
366368
given(messageConverter.read(eq(SimpleBean.class), isA(HttpInputMessage.class))).willReturn(simpleBean);
367369

368370
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
371+
369372
Object actualValue = resolver.resolveArgument(optionalRequestPart, mavContainer, webRequest, new ValidatingBinderFactory());
373+
assertEquals("Invalid argument value", Optional.of(simpleBean), actualValue);
374+
assertFalse("The requestHandled flag shouldn't change", mavContainer.isRequestHandled());
370375

376+
actualValue = resolver.resolveArgument(optionalRequestPart, mavContainer, webRequest, new ValidatingBinderFactory());
371377
assertEquals("Invalid argument value", Optional.of(simpleBean), actualValue);
372378
assertFalse("The requestHandled flag shouldn't change", mavContainer.isRequestHandled());
373379
}
@@ -378,8 +384,12 @@ public void resolveOptionalRequestPartNotPresent() throws Exception {
378384
given(messageConverter.read(eq(SimpleBean.class), isA(RequestPartServletServerHttpRequest.class))).willReturn(null);
379385

380386
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
387+
381388
Object actualValue = resolver.resolveArgument(optionalRequestPart, mavContainer, webRequest, new ValidatingBinderFactory());
389+
assertEquals("Invalid argument value", Optional.empty(), actualValue);
390+
assertFalse("The requestHandled flag shouldn't change", mavContainer.isRequestHandled());
382391

392+
actualValue = resolver.resolveArgument(optionalRequestPart, mavContainer, webRequest, new ValidatingBinderFactory());
383393
assertEquals("Invalid argument value", Optional.empty(), actualValue);
384394
assertFalse("The requestHandled flag shouldn't change", mavContainer.isRequestHandled());
385395
}
@@ -390,8 +400,8 @@ private void testResolveArgument(SimpleBean argValue, MethodParameter parameter)
390400
given(messageConverter.read(eq(SimpleBean.class), isA(HttpInputMessage.class))).willReturn(argValue);
391401

392402
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
393-
Object actualValue = resolver.resolveArgument(parameter, mavContainer, webRequest, new ValidatingBinderFactory());
394403

404+
Object actualValue = resolver.resolveArgument(parameter, mavContainer, webRequest, new ValidatingBinderFactory());
395405
assertEquals("Invalid argument value", argValue, actualValue);
396406
assertFalse("The requestHandled flag shouldn't change", mavContainer.isRequestHandled());
397407
}
@@ -425,22 +435,24 @@ public WebDataBinder createBinder(NativeWebRequest webRequest, Object target, St
425435
}
426436
}
427437

438+
428439
@SuppressWarnings("unused")
429-
public void handle(@RequestPart SimpleBean requestPart,
430-
@RequestPart(value="requestPart", required=false) SimpleBean namedRequestPart,
431-
@Valid @RequestPart("requestPart") SimpleBean validRequestPart,
432-
@RequestPart("requestPart") MultipartFile multipartFile,
433-
@RequestPart("requestPart") List<MultipartFile> multipartFileList,
434-
@RequestPart("requestPart") MultipartFile[] multipartFileArray,
435-
int i,
436-
MultipartFile multipartFileNotAnnot,
437-
Part part,
438-
@RequestPart("part") List<Part> partList,
439-
@RequestPart("part") Part[] partArray,
440-
@RequestParam MultipartFile requestParamAnnot,
441-
Optional<MultipartFile> optionalMultipartFile,
442-
Optional<Part> optionalPart,
443-
@RequestPart("requestPart") Optional<SimpleBean> optionalRequestPart) {
440+
public void handle(
441+
@RequestPart SimpleBean requestPart,
442+
@RequestPart(value="requestPart", required=false) SimpleBean namedRequestPart,
443+
@Valid @RequestPart("requestPart") SimpleBean validRequestPart,
444+
@RequestPart("requestPart") MultipartFile multipartFile,
445+
@RequestPart("requestPart") List<MultipartFile> multipartFileList,
446+
@RequestPart("requestPart") MultipartFile[] multipartFileArray,
447+
int i,
448+
MultipartFile multipartFileNotAnnot,
449+
Part part,
450+
@RequestPart("requestPart") List<Part> partList,
451+
@RequestPart("requestPart") Part[] partArray,
452+
@RequestParam MultipartFile requestParamAnnot,
453+
Optional<MultipartFile> optionalMultipartFile,
454+
Optional<Part> optionalPart,
455+
@RequestPart("requestPart") Optional<SimpleBean> optionalRequestPart) {
444456
}
445457

446458
}

0 commit comments

Comments
 (0)