SPR-8642 IMPROVE ERROR REPORTING WHEN RESOLVING MULTIPART REQUEST METHOD ARGUMENTS
Separate client from server errors as much as possible in this order: - raise MultipartException when resolving a multipart arg and the request is not a multipart request (400) - raise IllegalArgumentException when the arg type is MultipartFile but the request is not of type MultipartHttpServletRequest (500) - raise MissingServletRequestPartException when a MultipartResolver is in use but the part is not found (400) - detect presence of Servlet 3.0 before using standard multipart parsing to find a request part or raise IllegalArgumentException (500) Unfortunately it is not always possible to separate client from server errors mainly because the Servlet 3.0 API does not distinguish between the case of 0 request parts vs multipart processing not being configured.
This commit is contained in:
@@ -24,6 +24,7 @@ import java.util.Arrays;
|
||||
import org.junit.AfterClass;
|
||||
import org.junit.Before;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Test;
|
||||
import org.mortbay.jetty.Server;
|
||||
import org.mortbay.jetty.servlet.Context;
|
||||
@@ -52,6 +53,7 @@ import org.springframework.web.context.support.AnnotationConfigWebApplicationCon
|
||||
import org.springframework.web.multipart.MultipartFile;
|
||||
import org.springframework.web.multipart.MultipartResolver;
|
||||
import org.springframework.web.multipart.commons.CommonsMultipartResolver;
|
||||
import org.springframework.web.multipart.support.StandardServletMultipartResolver;
|
||||
import org.springframework.web.servlet.DispatcherServlet;
|
||||
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
|
||||
import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
|
||||
@@ -69,6 +71,7 @@ public class RequestPartIntegrationTests {
|
||||
|
||||
private static String baseUrl;
|
||||
|
||||
|
||||
@BeforeClass
|
||||
public static void startServer() throws Exception {
|
||||
|
||||
@@ -78,10 +81,19 @@ public class RequestPartIntegrationTests {
|
||||
server = new Server(port);
|
||||
Context context = new Context(server, "/");
|
||||
|
||||
Class<?> config = CommonsMultipartResolverTestConfig.class;
|
||||
ServletHolder commonsResolverServlet = new ServletHolder(DispatcherServlet.class);
|
||||
commonsResolverServlet.setInitParameter("contextConfigLocation", CommonsMultipartResolverTestConfig.class.getName());
|
||||
commonsResolverServlet.setInitParameter("contextConfigLocation", config.getName());
|
||||
commonsResolverServlet.setInitParameter("contextClass", AnnotationConfigWebApplicationContext.class.getName());
|
||||
context.addServlet(commonsResolverServlet, "/commons/*");
|
||||
context.addServlet(commonsResolverServlet, "/commons-resolver/*");
|
||||
|
||||
config = StandardMultipartResolverTestConfig.class;
|
||||
ServletHolder standardResolverServlet = new ServletHolder(DispatcherServlet.class);
|
||||
standardResolverServlet.setInitParameter("contextConfigLocation", config.getName());
|
||||
standardResolverServlet.setInitParameter("contextClass", AnnotationConfigWebApplicationContext.class.getName());
|
||||
context.addServlet(standardResolverServlet, "/standard-resolver/*");
|
||||
|
||||
// TODO: add Servlet 3.0 test case without MultipartResolver
|
||||
|
||||
server.start();
|
||||
}
|
||||
@@ -103,17 +115,28 @@ public class RequestPartIntegrationTests {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void commonsMultipartResolver() throws Exception {
|
||||
testCreate(baseUrl + "/commons-resolver/test");
|
||||
}
|
||||
|
||||
@Test
|
||||
@Ignore("jetty 6.1.9 doesn't support Servlet 3.0")
|
||||
public void standardMultipartResolver() throws Exception {
|
||||
testCreate(baseUrl + "/standard-resolver/test");
|
||||
}
|
||||
|
||||
private void testCreate(String url) {
|
||||
MultiValueMap<String, Object> parts = new LinkedMultiValueMap<String, Object>();
|
||||
HttpEntity<TestData> jsonEntity = new HttpEntity<TestData>(new TestData("Jason"));
|
||||
parts.add("json-data", jsonEntity);
|
||||
parts.add("file-data", new ClassPathResource("logo.jpg", this.getClass()));
|
||||
|
||||
URI location = restTemplate.postForLocation(baseUrl + "/commons/test", parts);
|
||||
URI location = restTemplate.postForLocation(url, parts);
|
||||
assertEquals("http://localhost:8080/test/Jason/logo.jpg", location.toString());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@Configuration
|
||||
@EnableWebMvc
|
||||
@@ -125,13 +148,22 @@ public class RequestPartIntegrationTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Configuration
|
||||
static class CommonsMultipartResolverTestConfig extends RequestPartTestConfig {
|
||||
|
||||
@Bean
|
||||
public MultipartResolver multipartResolver() {
|
||||
return new CommonsMultipartResolver();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Configuration
|
||||
static class StandardMultipartResolverTestConfig extends RequestPartTestConfig {
|
||||
|
||||
@Bean
|
||||
public MultipartResolver multipartResolver() {
|
||||
return new StandardServletMultipartResolver();
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
|
||||
@@ -61,7 +61,9 @@ import org.springframework.web.context.request.NativeWebRequest;
|
||||
import org.springframework.web.context.request.ServletWebRequest;
|
||||
import org.springframework.web.method.annotation.support.MethodArgumentNotValidException;
|
||||
import org.springframework.web.method.support.ModelAndViewContainer;
|
||||
import org.springframework.web.multipart.MultipartException;
|
||||
import org.springframework.web.multipart.MultipartFile;
|
||||
import org.springframework.web.multipart.support.MissingServletRequestPartException;
|
||||
import org.springframework.web.multipart.support.RequestPartServletServerHttpRequest;
|
||||
|
||||
/**
|
||||
@@ -171,6 +173,8 @@ public class RequestPartMethodArgumentResolverTests {
|
||||
public void resolveServlet30PartArgument() throws Exception {
|
||||
MockPart expected = new MockPart("servlet30Part", "Hello World".getBytes());
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.setMethod("POST");
|
||||
request.setContentType("multipart/form-data");
|
||||
request.addPart(expected);
|
||||
webRequest = new ServletWebRequest(request);
|
||||
|
||||
@@ -212,8 +216,8 @@ public class RequestPartMethodArgumentResolverTests {
|
||||
try {
|
||||
testResolveArgument(null, paramValidRequestPart);
|
||||
fail("Expected exception");
|
||||
} catch (ServletRequestBindingException e) {
|
||||
assertTrue(e.getMessage().contains("Missing request part"));
|
||||
} catch (MissingServletRequestPartException e) {
|
||||
assertEquals("requestPart", e.getRequestPartName());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -221,19 +225,26 @@ public class RequestPartMethodArgumentResolverTests {
|
||||
public void resolveRequestPartNotRequired() throws Exception {
|
||||
testResolveArgument(new SimpleBean("foo"), paramValidRequestPart);
|
||||
}
|
||||
|
||||
@Test(expected=MultipartException.class)
|
||||
public void notMultipartRequest() throws Exception {
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
resolver.resolveArgument(paramMultipartFile, new ModelAndViewContainer(), new ServletWebRequest(request), null);
|
||||
fail("Expected exception");
|
||||
}
|
||||
|
||||
private void testResolveArgument(SimpleBean expectedValue, MethodParameter parameter) throws IOException, Exception {
|
||||
private void testResolveArgument(SimpleBean argValue, MethodParameter parameter) throws IOException, Exception {
|
||||
MediaType contentType = MediaType.TEXT_PLAIN;
|
||||
multipartRequest.addHeader("Content-Type", contentType.toString());
|
||||
|
||||
expect(messageConverter.canRead(SimpleBean.class, contentType)).andReturn(true);
|
||||
expect(messageConverter.read(eq(SimpleBean.class), isA(RequestPartServletServerHttpRequest.class))).andReturn(expectedValue);
|
||||
expect(messageConverter.read(eq(SimpleBean.class), isA(RequestPartServletServerHttpRequest.class))).andReturn(argValue);
|
||||
replay(messageConverter);
|
||||
|
||||
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
|
||||
Object actualValue = resolver.resolveArgument(parameter, mavContainer, webRequest, new ValidatingBinderFactory());
|
||||
|
||||
assertEquals("Invalid argument value", expectedValue, actualValue);
|
||||
assertEquals("Invalid argument value", argValue, actualValue);
|
||||
assertTrue("The ResolveView flag shouldn't change", mavContainer.isResolveView());
|
||||
|
||||
verify(messageConverter);
|
||||
|
||||
@@ -38,6 +38,7 @@ import org.springframework.web.HttpRequestMethodNotSupportedException;
|
||||
import org.springframework.web.bind.MissingServletRequestParameterException;
|
||||
import org.springframework.web.bind.ServletRequestBindingException;
|
||||
import org.springframework.web.method.annotation.support.MethodArgumentNotValidException;
|
||||
import org.springframework.web.multipart.support.MissingServletRequestPartException;
|
||||
import org.springframework.web.servlet.ModelAndView;
|
||||
import org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException;
|
||||
|
||||
@@ -147,6 +148,15 @@ public class DefaultHandlerExceptionResolverTests {
|
||||
assertEquals("Invalid status code", 400, response.getStatus());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void handleMissingServletRequestPartException() throws Exception {
|
||||
MissingServletRequestPartException ex = new MissingServletRequestPartException("name");
|
||||
ModelAndView mav = exceptionResolver.resolveException(request, response, null, ex);
|
||||
assertNotNull("No ModelAndView returned", mav);
|
||||
assertTrue("No Empty ModelAndView returned", mav.isEmpty());
|
||||
assertEquals("Invalid status code", 400, response.getStatus());
|
||||
}
|
||||
|
||||
public void handle(String arg) {
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user