SPR-5732 - When no type conversion strategy is found on a @Controller handler method bind target, a 500 error code should be returned not a 400.

This commit is contained in:
Arjen Poutsma 2009-05-11 14:52:14 +00:00
parent 800af734d9
commit bf7a947559
8 changed files with 121 additions and 8 deletions

View File

@ -355,6 +355,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
} }
catch (IllegalArgumentException ex) { catch (IllegalArgumentException ex) {
throw new TypeMismatchException(value, requiredType, ex); throw new TypeMismatchException(value, requiredType, ex);
} catch (IllegalStateException ex) {
throw new ConversionNotSupportedException(value, requiredType, ex);
} }
} }
@ -382,6 +384,11 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, null, value); new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, null, value);
throw new TypeMismatchException(pce, pd.getPropertyType(), ex); throw new TypeMismatchException(pce, pd.getPropertyType(), ex);
} }
catch (IllegalStateException ex) {
PropertyChangeEvent pce =
new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, null, value);
throw new ConversionNotSupportedException(pce, pd.getPropertyType(), ex);
}
} }
@ -840,6 +847,11 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra
new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue()); new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue());
throw new TypeMismatchException(pce, pd.getPropertyType(), ex); throw new TypeMismatchException(pce, pd.getPropertyType(), ex);
} }
catch (IllegalStateException ex) {
PropertyChangeEvent pce =
new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue());
throw new ConversionNotSupportedException(pce, pd.getPropertyType(), ex);
}
catch (IllegalAccessException ex) { catch (IllegalAccessException ex) {
PropertyChangeEvent pce = PropertyChangeEvent pce =
new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue()); new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, pv.getValue());

View File

@ -0,0 +1,44 @@
/*
* Copyright 2002-2009 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.beans;
import java.beans.PropertyChangeEvent;
/**
* Exception thrown when no suitable editor can be found to set a bean property.
*
* @author Arjen Poutsma
* @since 3.0
*/
public class ConversionNotSupportedException extends TypeMismatchException {
public ConversionNotSupportedException(PropertyChangeEvent propertyChangeEvent, Class requiredType) {
super(propertyChangeEvent, requiredType);
}
public ConversionNotSupportedException(PropertyChangeEvent propertyChangeEvent, Class requiredType, Throwable cause) {
super(propertyChangeEvent, requiredType, cause);
}
public ConversionNotSupportedException(Object value, Class requiredType) {
super(value, requiredType);
}
public ConversionNotSupportedException(Object value, Class requiredType, Throwable cause) {
super(value, requiredType, cause);
}
}

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2008 the original author or authors. * Copyright 2002-2009 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -124,6 +124,10 @@ public class DirectFieldAccessor extends AbstractPropertyAccessor {
PropertyChangeEvent pce = new PropertyChangeEvent(this.target, propertyName, oldValue, newValue); PropertyChangeEvent pce = new PropertyChangeEvent(this.target, propertyName, oldValue, newValue);
throw new TypeMismatchException(pce, field.getType(), ex); throw new TypeMismatchException(pce, field.getType(), ex);
} }
catch (IllegalStateException ex) {
PropertyChangeEvent pce = new PropertyChangeEvent(this.target, propertyName, oldValue, newValue);
throw new ConversionNotSupportedException(pce, field.getType(), ex);
}
} }
public Object convertIfNecessary( public Object convertIfNecessary(
@ -134,6 +138,9 @@ public class DirectFieldAccessor extends AbstractPropertyAccessor {
catch (IllegalArgumentException ex) { catch (IllegalArgumentException ex) {
throw new TypeMismatchException(value, requiredType, ex); throw new TypeMismatchException(value, requiredType, ex);
} }
catch (IllegalStateException ex) {
throw new ConversionNotSupportedException(value, requiredType, ex);
}
} }
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2006 the original author or authors. * Copyright 2002-2009 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -49,6 +49,9 @@ public class SimpleTypeConverter extends PropertyEditorRegistrySupport implement
catch (IllegalArgumentException ex) { catch (IllegalArgumentException ex) {
throw new TypeMismatchException(value, requiredType, ex); throw new TypeMismatchException(value, requiredType, ex);
} }
catch (IllegalStateException ex) {
throw new ConversionNotSupportedException(value, requiredType, ex);
}
} }
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2008 the original author or authors. * Copyright 2002-2009 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -209,7 +209,7 @@ class TypeConverterDelegate {
} }
if (!ClassUtils.isAssignableValue(requiredType, convertedValue)) { if (!ClassUtils.isAssignableValue(requiredType, convertedValue)) {
// Definitely doesn't match: throw IllegalArgumentException. // Definitely doesn't match: throw IllegalArgumentException/IllegalStateException
StringBuilder msg = new StringBuilder(); StringBuilder msg = new StringBuilder();
msg.append("Cannot convert value of type [").append(ClassUtils.getDescriptiveType(newValue)); msg.append("Cannot convert value of type [").append(ClassUtils.getDescriptiveType(newValue));
msg.append("] to required type [").append(ClassUtils.getQualifiedName(requiredType)).append("]"); msg.append("] to required type [").append(ClassUtils.getQualifiedName(requiredType)).append("]");
@ -219,11 +219,12 @@ class TypeConverterDelegate {
if (editor != null) { if (editor != null) {
msg.append(": PropertyEditor [").append(editor.getClass().getName()).append( msg.append(": PropertyEditor [").append(editor.getClass().getName()).append(
"] returned inappropriate value"); "] returned inappropriate value");
throw new IllegalArgumentException(msg.toString());
} }
else { else {
msg.append(": no matching editors or conversion strategy found"); msg.append(": no matching editors or conversion strategy found");
throw new IllegalStateException(msg.toString());
} }
throw new IllegalArgumentException(msg.toString());
} }
} }

View File

@ -23,6 +23,7 @@ import javax.servlet.http.HttpServletResponse;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.beans.ConversionNotSupportedException;
import org.springframework.beans.TypeMismatchException; import org.springframework.beans.TypeMismatchException;
import org.springframework.core.Ordered; import org.springframework.core.Ordered;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
@ -95,6 +96,9 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
return handleMissingServletRequestParameter((MissingServletRequestParameterException) ex, request, return handleMissingServletRequestParameter((MissingServletRequestParameterException) ex, request,
response, handler); response, handler);
} }
else if (ex instanceof ConversionNotSupportedException) {
return handleConversionNotSupported((ConversionNotSupportedException) ex, request, response, handler);
}
else if (ex instanceof TypeMismatchException) { else if (ex instanceof TypeMismatchException) {
return handleTypeMismatch((TypeMismatchException) ex, request, response, handler); return handleTypeMismatch((TypeMismatchException) ex, request, response, handler);
} }
@ -211,6 +215,27 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
return new ModelAndView(); return new ModelAndView();
} }
/**
* Handle the case when a {@link org.springframework.web.bind.WebDataBinder} conversion cannot occur. <p>The default
* implementation sends an HTTP 500 error, and returns an empty {@code ModelAndView}. Alternatively, a fallback view
* could be chosen, or the TypeMismatchException could be rethrown as-is.
*
* @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
*/
protected ModelAndView handleConversionNotSupported(ConversionNotSupportedException ex,
HttpServletRequest request,
HttpServletResponse response,
Object handler) throws Exception {
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
return new ModelAndView();
}
/** /**
* Handle the case when a {@link org.springframework.web.bind.WebDataBinder} conversion error occurs. <p>The default * Handle the case when a {@link org.springframework.web.bind.WebDataBinder} conversion error occurs. <p>The default
* implementation sends an HTTP 400 error, and returns an empty {@code ModelAndView}. Alternatively, a fallback view * implementation sends an HTTP 400 error, and returns an empty {@code ModelAndView}. Alternatively, a fallback view
@ -228,7 +253,6 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
HttpServletRequest request, HttpServletRequest request,
HttpServletResponse response, HttpServletResponse response,
Object handler) throws Exception { Object handler) throws Exception {
response.sendError(HttpServletResponse.SC_BAD_REQUEST); response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return new ModelAndView(); return new ModelAndView();
} }

View File

@ -17,8 +17,8 @@
package org.springframework.web.servlet.mvc.annotation; package org.springframework.web.servlet.mvc.annotation;
import java.io.IOException; import java.io.IOException;
import java.io.Writer;
import java.io.Serializable; import java.io.Serializable;
import java.io.Writer;
import java.lang.annotation.ElementType; import java.lang.annotation.ElementType;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
@ -106,6 +106,7 @@ import org.springframework.web.util.NestedServletException;
/** /**
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Sam Brannen * @author Sam Brannen
* @author Arjen Poutsma
* @since 2.5 * @since 2.5
*/ */
public class ServletAnnotationControllerTests { public class ServletAnnotationControllerTests {

View File

@ -74,7 +74,18 @@ public class UriTemplateServletAnnotationControllerTests {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-11-18"); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-11-18");
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
servlet.service(request, response); servlet.service(request, response);
assertEquals("test-42", response.getContentAsString()); assertEquals(200, response.getStatus());
request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-foo-bar");
response = new MockHttpServletResponse();
servlet.service(request, response);
assertEquals(400, response.getStatus());
initServlet(NonBindingUriTemplateController.class);
request = new MockHttpServletRequest("GET", "/hotels/42/dates/2008-foo-bar");
response = new MockHttpServletResponse();
servlet.service(request, response);
assertEquals(500, response.getStatus());
} }
@Test @Test
@ -276,6 +287,16 @@ public class UriTemplateServletAnnotationControllerTests {
} }
@Controller
public static class NonBindingUriTemplateController {
@RequestMapping("/hotels/{hotel}/dates/{date}")
public void handle(@PathVariable("hotel") String hotel, @PathVariable Date date, Writer writer)
throws IOException {
}
}
@Controller @Controller
@RequestMapping("/hotels/{hotel}") @RequestMapping("/hotels/{hotel}")
public static class RelativePathUriTemplateController { public static class RelativePathUriTemplateController {