diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java index 2919bc14d98..4596ddf9e15 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolver.java @@ -21,6 +21,7 @@ import java.util.Map; import javax.servlet.ServletException; import org.springframework.core.MethodParameter; +import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.ValueConstants; @@ -70,8 +71,10 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod } @Override - protected void handleMissingValue(String name, MethodParameter parameter) throws ServletException { - throw new IllegalStateException("Could not find the URL template variable [" + name + "]"); + protected void handleMissingValue(String name, MethodParameter param) throws ServletRequestBindingException { + String paramType = param.getParameterType().getName(); + throw new ServletRequestBindingException( + "Missing URI template variable '" + name + "' for method parameter type [" + paramType + "]"); } @Override diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java index 07088f12e0b..f9566f1de8c 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java @@ -36,6 +36,7 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.HttpRequestMethodNotSupportedException; import org.springframework.web.bind.MissingServletRequestParameterException; +import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver; import org.springframework.web.servlet.mvc.method.annotation.support.RequestBodyNotValidException; @@ -55,9 +56,11 @@ import org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMeth * @see #handleHttpRequestMethodNotSupported * @see #handleHttpMediaTypeNotSupported * @see #handleMissingServletRequestParameter + * @see #handleServletRequestBindingException * @see #handleTypeMismatch * @see #handleHttpMessageNotReadable * @see #handleHttpMessageNotWritable + * @see #handleRequestBodyNotValidException */ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionResolver { @@ -107,6 +110,10 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes return handleMissingServletRequestParameter((MissingServletRequestParameterException) ex, request, response, handler); } + else if (ex instanceof ServletRequestBindingException) { + return handleServletRequestBindingException((ServletRequestBindingException) ex, request, response, + handler); + } else if (ex instanceof ConversionNotSupportedException) { return handleConversionNotSupported((ConversionNotSupportedException) ex, request, response, handler); } @@ -139,11 +146,11 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param response current HTTP response * @param handler the executed handler, or null if none chosen * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleNoSuchRequestHandlingMethod(NoSuchRequestHandlingMethodException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { pageNotFoundLogger.warn(ex.getMessage()); response.sendError(HttpServletResponse.SC_NOT_FOUND); @@ -160,11 +167,11 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param response current HTTP response * @param handler the executed handler, or null if none chosen * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleHttpRequestMethodNotSupported(HttpRequestMethodNotSupportedException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { pageNotFoundLogger.warn(ex.getMessage()); String[] supportedMethods = ex.getSupportedMethods(); @@ -183,13 +190,12 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the HttpMediaTypeNotSupportedException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleHttpMediaTypeNotSupported(HttpMediaTypeNotSupportedException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE); List mediaTypes = ex.getSupportedMediaTypes(); @@ -208,13 +214,12 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the HttpMediaTypeNotAcceptableException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleHttpMediaTypeNotAcceptable(HttpMediaTypeNotAcceptableException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_NOT_ACCEPTABLE); return new ModelAndView(); @@ -228,13 +233,30 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the MissingServletRequestParameterException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleMissingServletRequestParameter(MissingServletRequestParameterException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { + + response.sendError(HttpServletResponse.SC_BAD_REQUEST); + return new ModelAndView(); + } + + /** + * Handle the case when an unrecoverable binding exception occurs - e.g. required header, required cookie. + *

The default implementation sends an HTTP 400 error, and returns an empty {@code ModelAndView}. + * Alternatively, a fallback view could be chosen, or the exception could be rethrown as-is. + * @param ex the exception to be handled + * @param request current HTTP request + * @param response current HTTP response + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() + */ + protected ModelAndView handleServletRequestBindingException(ServletRequestBindingException ex, + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_BAD_REQUEST); return new ModelAndView(); @@ -247,13 +269,12 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the ConversionNotSupportedException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleConversionNotSupported(ConversionNotSupportedException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); return new ModelAndView(); @@ -266,13 +287,12 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the TypeMismatchException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleTypeMismatch(TypeMismatchException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_BAD_REQUEST); return new ModelAndView(); @@ -287,13 +307,12 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the HttpMessageNotReadableException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleHttpMessageNotReadable(HttpMessageNotReadableException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_BAD_REQUEST); return new ModelAndView(); @@ -308,13 +327,12 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * @param ex the HttpMessageNotWritableException to be handled * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleHttpMessageNotWritable(HttpMessageNotWritableException ex, - HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { + HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); return new ModelAndView(); @@ -325,10 +343,9 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes * implementation sends an HTTP 400 error along with a message containing the errors. * @param request current HTTP request * @param response current HTTP response - * @param handler the executed handler, or null if none chosen - * at the time of the exception (for example, if multipart resolution failed) - * @return a ModelAndView to render, or null if handled directly - * @throws Exception an Exception that should be thrown as result of the servlet request + * @param handler the executed handler + * @return an empty ModelAndView indicating the exception was handled + * @throws IOException potentially thrown from response.sendError() */ protected ModelAndView handleRequestBodyNotValidException(RequestBodyNotValidException ex, HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java index 3f81ca2be8a..b0386b5f945 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java @@ -885,14 +885,8 @@ public class ServletAnnotationControllerTests { MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/myPath.do"); request.addParameter("view", "other"); MockHttpServletResponse response = new MockHttpServletResponse(); - try { - servlet.service(request, response); - fail("Should have failed because of type-level parameter constraint not met"); - } - catch (ServletException ex) { - // expected - ex.printStackTrace(); - } + servlet.service(request, response); + assertEquals(400, response.getStatus()); request = new MockHttpServletRequest(servletContext, "GET", "/myPath.do"); request.addParameter("active", "true"); @@ -905,14 +899,8 @@ public class ServletAnnotationControllerTests { request.addParameter("view", "my"); request.addParameter("lang", "de"); response = new MockHttpServletResponse(); - try { - servlet.service(request, response); - fail("Should have failed because of type-level parameter constraint not met"); - } - catch (ServletException ex) { - // expected - ex.printStackTrace(); - } + servlet.service(request, response); + assertEquals(400, response.getStatus()); request = new MockHttpServletRequest(servletContext, "GET", "/myPath.do"); request.addParameter("view", "my"); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolverTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolverTests.java index 22ddfd9c7ad..c455267500f 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolverTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/support/PathVariableMethodArgumentResolverTests.java @@ -30,6 +30,7 @@ import org.junit.Test; import org.springframework.core.MethodParameter; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.method.support.ModelAndViewContainer; @@ -85,7 +86,7 @@ public class PathVariableMethodArgumentResolverTests { assertEquals("PathVariable not added to the model", "value", mavContainer.getAttribute("name")); } - @Test(expected = IllegalStateException.class) + @Test(expected = ServletRequestBindingException.class) public void handleMissingValue() throws Exception { resolver.resolveArgument(paramNamedString, mavContainer, webRequest, null); fail("Unresolved path variable should lead to exception."); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java index c18387933ba..53aa6c22ed3 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java @@ -35,6 +35,7 @@ import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.HttpRequestMethodNotSupportedException; import org.springframework.web.bind.MissingServletRequestParameterException; +import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.mvc.method.annotation.support.RequestBodyNotValidException; import org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException; @@ -96,6 +97,16 @@ public class DefaultHandlerExceptionResolverTests { assertEquals("Invalid status code", 400, response.getStatus()); } + @Test + public void handleServletRequestBindingException() { + String message = "Missing required value - header, cookie, or pathvar"; + ServletRequestBindingException ex = new ServletRequestBindingException(message); + 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()); + } + @Test public void handleTypeMismatch() { TypeMismatchException ex = new TypeMismatchException("foo", String.class);