SPR-8429 Return 400 instead of 500 for required header/cookie/pathvar. This is also more in line with jax-rs.

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4601 50f2f4bb-b051-0410-bef5-90022cba6387

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 {
+	protected void handleMissingValue(String name, MethodParameter param) throws ServletRequestBindingException {
-		throw new IllegalStateException("Could not find the URL template variable [" + name + "]");
+		String paramType = param.getParameterType().getName();
+		throw new ServletRequestBindingException(
+				"Missing URI template variable '" + name + "' for method parameter type [" + paramType + "]");
 	}
 
 	@Override

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 <code>null</code> if none chosen
 	 * at the time of the exception (for example, if multipart resolution failed)
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
+	 * @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 <code>null</code> if none chosen
 	 * at the time of the exception (for example, if multipart resolution failed)
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
+	 * @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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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<MediaType> 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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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.
+	 * <p>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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	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 <code>null</code> if none chosen
+	 * @param handler the executed handler
-	 * at the time of the exception (for example, if multipart resolution failed)
+	 * @return an empty ModelAndView indicating the exception was handled
-	 * @return a ModelAndView to render, or <code>null</code> if handled directly
+	 * @throws IOException potentially thrown from response.sendError()
-	 * @throws Exception an Exception that should be thrown as result of the servlet request
 	 */
 	protected ModelAndView handleRequestBodyNotValidException(RequestBodyNotValidException ex,
 			HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException {

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");
+		assertEquals(400, response.getStatus());
-		}
-		catch (ServletException ex) {
-			// expected
-			ex.printStackTrace();
-		}
 
 		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");
+		assertEquals(400, response.getStatus());
-		}
-		catch (ServletException ex) {
-			// expected
-			ex.printStackTrace();
-		}
 
 		request = new MockHttpServletRequest(servletContext, "GET", "/myPath.do");
 		request.addParameter("view", "my");

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.");

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);