SPR-8429 Return 400 instead of 500 for required header/cookie/pathvar. This is also more in line with jax-rs.
This commit is contained in:
parent
41e5d55d52
commit
5f76ad809f
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
* @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 <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
|
||||
* @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 <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
|
||||
* @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<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
|
||||
* at the time of the exception (for example, if multipart resolution failed)
|
||||
* @return a ModelAndView to render, or <code>null</code> 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 <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
|
||||
* @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.
|
||||
* <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
|
||||
* at the time of the exception (for example, if multipart resolution failed)
|
||||
* @return a ModelAndView to render, or <code>null</code> 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 <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
|
||||
* @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 <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
|
||||
* @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 <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
|
||||
* @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 <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
|
||||
* @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 {
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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.");
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue