Missing path variable is now a 500 error
Before this change a missing path variable value resulted in a 400 error where in fact the error is due to a mismatch between the declared @PathVariable and the URI template, i.e. a 500 error. This change introduced a MissingPathVariableException as a sub-class of ServletRequestBindingException (the exception previously thrown) and results in a response status code of 500 by default. Issue: SPR-13121
This commit is contained in:
		
							parent
							
								
									7b365583e3
								
							
						
					
					
						commit
						4b05bda0bf
					
				|  | @ -0,0 +1,71 @@ | |||
| /* | ||||
|  * Copyright 2002-2015 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.web.bind; | ||||
| 
 | ||||
| import org.springframework.core.MethodParameter; | ||||
| 
 | ||||
| /** | ||||
|  * {@link ServletRequestBindingException} subclass that indicates that a path | ||||
|  * variable expected in the method parameters of an {@code @RequestMapping} | ||||
|  * method is not present among the URI variables extracted from the URL. | ||||
|  * Typically that means the URI template does not match the path variable name | ||||
|  * declared on the method parameter. | ||||
|  * | ||||
|  * @author Rossen Stoyanchev | ||||
|  * @since 4.2 | ||||
|  */ | ||||
| @SuppressWarnings("serial") | ||||
| public class MissingPathVariableException extends ServletRequestBindingException { | ||||
| 
 | ||||
| 	private final String variableName; | ||||
| 
 | ||||
| 	private final MethodParameter parameter; | ||||
| 
 | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Constructor for MissingPathVariableException. | ||||
| 	 * @param variableName the name of the missing path variable | ||||
| 	 * @param parameter the method parameter | ||||
| 	 */ | ||||
| 	public MissingPathVariableException(String variableName, MethodParameter parameter) { | ||||
| 		super(""); | ||||
| 		this.variableName = variableName; | ||||
| 		this.parameter = parameter; | ||||
| 	} | ||||
| 
 | ||||
| 
 | ||||
| 	@Override | ||||
| 	public String getMessage() { | ||||
| 		return "Missing URI template variable '" + this.variableName + | ||||
| 				"' for method parameter of type " + parameter.getParameterType().getSimpleName(); | ||||
| 	} | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Return the expected name of the path variable. | ||||
| 	 */ | ||||
| 	public final String getVariableName() { | ||||
| 		return this.variableName; | ||||
| 	} | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Return the method parameter bound to the path variable. | ||||
| 	 */ | ||||
| 	public final MethodParameter getParameter() { | ||||
| 		return this.parameter; | ||||
| 	} | ||||
| 
 | ||||
| } | ||||
|  | @ -1,5 +1,5 @@ | |||
| /* | ||||
|  * Copyright 2002-2014 the original author or authors. | ||||
|  * Copyright 2002-2015 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. | ||||
|  | @ -25,6 +25,7 @@ import org.springframework.core.convert.ConversionService; | |||
| import org.springframework.core.convert.TypeDescriptor; | ||||
| import org.springframework.core.convert.converter.Converter; | ||||
| import org.springframework.util.StringUtils; | ||||
| import org.springframework.web.bind.MissingPathVariableException; | ||||
| import org.springframework.web.bind.ServletRequestBindingException; | ||||
| import org.springframework.web.bind.WebDataBinder; | ||||
| import org.springframework.web.bind.annotation.PathVariable; | ||||
|  | @ -101,9 +102,10 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod | |||
| 	} | ||||
| 
 | ||||
| 	@Override | ||||
| 	protected void handleMissingValue(String name, MethodParameter parameter) throws ServletRequestBindingException { | ||||
| 		throw new ServletRequestBindingException("Missing URI template variable '" + name + | ||||
| 				"' for method parameter of type " + parameter.getParameterType().getSimpleName()); | ||||
| 	protected void handleMissingValue(String name, MethodParameter parameter) | ||||
| 			throws ServletRequestBindingException { | ||||
| 
 | ||||
| 		throw new MissingPathVariableException(name, parameter); | ||||
| 	} | ||||
| 
 | ||||
| 	@Override | ||||
|  |  | |||
|  | @ -1,5 +1,5 @@ | |||
| /* | ||||
|  * Copyright 2002-2013 the original author or authors. | ||||
|  * Copyright 2002-2015 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. | ||||
|  | @ -37,6 +37,7 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException; | |||
| import org.springframework.web.HttpMediaTypeNotSupportedException; | ||||
| import org.springframework.web.HttpRequestMethodNotSupportedException; | ||||
| import org.springframework.web.bind.MethodArgumentNotValidException; | ||||
| import org.springframework.web.bind.MissingPathVariableException; | ||||
| import org.springframework.web.bind.MissingServletRequestParameterException; | ||||
| import org.springframework.web.bind.ServletRequestBindingException; | ||||
| import org.springframework.web.bind.annotation.ControllerAdvice; | ||||
|  | @ -96,6 +97,7 @@ public abstract class ResponseEntityExceptionHandler { | |||
| 			HttpRequestMethodNotSupportedException.class, | ||||
| 			HttpMediaTypeNotSupportedException.class, | ||||
| 			HttpMediaTypeNotAcceptableException.class, | ||||
| 			MissingPathVariableException.class, | ||||
| 			MissingServletRequestParameterException.class, | ||||
| 			ServletRequestBindingException.class, | ||||
| 			ConversionNotSupportedException.class, | ||||
|  | @ -127,6 +129,10 @@ public abstract class ResponseEntityExceptionHandler { | |||
| 			HttpStatus status = HttpStatus.NOT_ACCEPTABLE; | ||||
| 			return handleHttpMediaTypeNotAcceptable((HttpMediaTypeNotAcceptableException) ex, headers, status, request); | ||||
| 		} | ||||
| 		else if (ex instanceof MissingPathVariableException) { | ||||
| 			HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR; | ||||
| 			return handleMissingPathVariable((MissingPathVariableException) ex, headers, status, request); | ||||
| 		} | ||||
| 		else if (ex instanceof MissingServletRequestParameterException) { | ||||
| 			HttpStatus status = HttpStatus.BAD_REQUEST; | ||||
| 			return handleMissingServletRequestParameter((MissingServletRequestParameterException) ex, headers, status, request); | ||||
|  | @ -270,6 +276,22 @@ public abstract class ResponseEntityExceptionHandler { | |||
| 		return handleExceptionInternal(ex, null, headers, status, request); | ||||
| 	} | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Customize the response for MissingPathVariableException. | ||||
| 	 * This method delegates to {@link #handleExceptionInternal(Exception, Object, HttpHeaders, HttpStatus, WebRequest)}. | ||||
| 	 * @param ex the exception | ||||
| 	 * @param headers the headers to be written to the response | ||||
| 	 * @param status the selected response status | ||||
| 	 * @param request the current request | ||||
| 	 * @return a {@code ResponseEntity} instance | ||||
| 	 * @since 4.2 | ||||
| 	 */ | ||||
| 	protected ResponseEntity<Object> handleMissingPathVariable(MissingPathVariableException ex, | ||||
| 			HttpHeaders headers, HttpStatus status, WebRequest request) { | ||||
| 
 | ||||
| 		return handleExceptionInternal(ex, null, headers, status, request); | ||||
| 	} | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Customize the response for MissingServletRequestParameterException. | ||||
| 	 * This method delegates to {@link #handleExceptionInternal(Exception, Object, HttpHeaders, HttpStatus, WebRequest)}. | ||||
|  |  | |||
|  | @ -1,5 +1,5 @@ | |||
| /* | ||||
|  * Copyright 2002-2013 the original author or authors. | ||||
|  * Copyright 2002-2015 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. | ||||
|  | @ -38,6 +38,7 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException; | |||
| import org.springframework.web.HttpMediaTypeNotSupportedException; | ||||
| import org.springframework.web.HttpRequestMethodNotSupportedException; | ||||
| import org.springframework.web.bind.MethodArgumentNotValidException; | ||||
| import org.springframework.web.bind.MissingPathVariableException; | ||||
| import org.springframework.web.bind.MissingServletRequestParameterException; | ||||
| import org.springframework.web.bind.ServletRequestBindingException; | ||||
| import org.springframework.web.bind.annotation.ModelAttribute; | ||||
|  | @ -119,6 +120,10 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes | |||
| 				return handleHttpMediaTypeNotAcceptable((HttpMediaTypeNotAcceptableException) ex, request, response, | ||||
| 						handler); | ||||
| 			} | ||||
| 			else if (ex instanceof MissingPathVariableException) { | ||||
| 				return handleMissingPathVariable((MissingPathVariableException) ex, request, | ||||
| 						response, handler); | ||||
| 			} | ||||
| 			else if (ex instanceof MissingServletRequestParameterException) { | ||||
| 				return handleMissingServletRequestParameter((MissingServletRequestParameterException) ex, request, | ||||
| 						response, handler); | ||||
|  | @ -247,6 +252,26 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes | |||
| 		return new ModelAndView(); | ||||
| 	} | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Handle the case when a declared path variable does not match any extracted URI variable. | ||||
| 	 * <p>The default implementation sends an HTTP 500 error, and returns an empty {@code ModelAndView}. | ||||
| 	 * Alternatively, a fallback view could be chosen, or the MissingPathVariableException | ||||
| 	 * could be rethrown as-is. | ||||
| 	 * @param ex the MissingPathVariableException 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() | ||||
| 	 * @since 4.2 | ||||
| 	 */ | ||||
| 	protected ModelAndView handleMissingPathVariable(MissingPathVariableException ex, | ||||
| 			HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException { | ||||
| 
 | ||||
| 		response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex.getMessage()); | ||||
| 		return new ModelAndView(); | ||||
| 	} | ||||
| 
 | ||||
| 	/** | ||||
| 	 * Handle the case when a required parameter is missing. | ||||
| 	 * <p>The default implementation sends an HTTP 400 error, and returns an empty {@code ModelAndView}. | ||||
|  |  | |||
|  | @ -1,5 +1,5 @@ | |||
| /* | ||||
|  * Copyright 2002-2012 the original author or authors. | ||||
|  * Copyright 2002-2015 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. | ||||
|  | @ -16,6 +16,8 @@ | |||
| 
 | ||||
| package org.springframework.web.servlet.mvc.method.annotation; | ||||
| 
 | ||||
| import static org.junit.Assert.*; | ||||
| 
 | ||||
| import java.lang.reflect.Method; | ||||
| import java.util.HashMap; | ||||
| import java.util.Map; | ||||
|  | @ -26,15 +28,13 @@ import org.junit.Test; | |||
| import org.springframework.core.MethodParameter; | ||||
| import org.springframework.mock.web.test.MockHttpServletRequest; | ||||
| import org.springframework.mock.web.test.MockHttpServletResponse; | ||||
| import org.springframework.web.bind.ServletRequestBindingException; | ||||
| import org.springframework.web.bind.MissingPathVariableException; | ||||
| import org.springframework.web.bind.annotation.PathVariable; | ||||
| import org.springframework.web.context.request.ServletWebRequest; | ||||
| import org.springframework.web.method.support.ModelAndViewContainer; | ||||
| import org.springframework.web.servlet.HandlerMapping; | ||||
| import org.springframework.web.servlet.View; | ||||
| 
 | ||||
| import static org.junit.Assert.*; | ||||
| 
 | ||||
| /** | ||||
|  * Test fixture with {@link PathVariableMethodArgumentResolver}. | ||||
|  * | ||||
|  | @ -96,7 +96,7 @@ public class PathVariableMethodArgumentResolverTests { | |||
| 		uriTemplateVars.put("name", "value"); | ||||
| 		request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); | ||||
| 
 | ||||
| 		Map<String, Object> pathVars = new HashMap<String, Object>(); | ||||
| 		Map<String, Object> pathVars; | ||||
| 		uriTemplateVars.put("oldName", "oldValue"); | ||||
| 		request.setAttribute(View.PATH_VARIABLES, uriTemplateVars); | ||||
| 
 | ||||
|  | @ -110,12 +110,13 @@ public class PathVariableMethodArgumentResolverTests { | |||
| 		assertEquals("oldValue", pathVars.get("oldName")); | ||||
| 	} | ||||
| 
 | ||||
| 	@Test(expected = ServletRequestBindingException.class) | ||||
| 	@Test(expected = MissingPathVariableException.class) | ||||
| 	public void handleMissingValue() throws Exception { | ||||
| 		resolver.resolveArgument(paramNamedString, mavContainer, webRequest, null); | ||||
| 		fail("Unresolved path variable should lead to exception."); | ||||
| 	} | ||||
| 
 | ||||
| 	@SuppressWarnings("unused") | ||||
| 	public void handle(@PathVariable(value = "name") String param1, String param2) { | ||||
| 	} | ||||
| 
 | ||||
|  |  | |||
|  | @ -26,6 +26,7 @@ import org.junit.Test; | |||
| 
 | ||||
| import org.springframework.beans.ConversionNotSupportedException; | ||||
| import org.springframework.beans.TypeMismatchException; | ||||
| import org.springframework.core.MethodParameter; | ||||
| import org.springframework.http.HttpHeaders; | ||||
| import org.springframework.http.HttpMethod; | ||||
| import org.springframework.http.HttpStatus; | ||||
|  | @ -41,6 +42,7 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException; | |||
| import org.springframework.web.HttpMediaTypeNotSupportedException; | ||||
| import org.springframework.web.HttpRequestMethodNotSupportedException; | ||||
| import org.springframework.web.bind.MethodArgumentNotValidException; | ||||
| import org.springframework.web.bind.MissingPathVariableException; | ||||
| import org.springframework.web.bind.MissingServletRequestParameterException; | ||||
| import org.springframework.web.bind.ServletRequestBindingException; | ||||
| import org.springframework.web.bind.annotation.ControllerAdvice; | ||||
|  | @ -86,15 +88,16 @@ public class ResponseEntityExceptionHandlerTests { | |||
| 	@Test | ||||
| 	public void supportsAllDefaultHandlerExceptionResolverExceptionTypes() throws Exception { | ||||
| 
 | ||||
| 		Method annotMethod = ResponseEntityExceptionHandler.class.getMethod("handleException", Exception.class, WebRequest.class); | ||||
| 		ExceptionHandler annot = annotMethod.getAnnotation(ExceptionHandler.class); | ||||
| 		List<Class<? extends Throwable>> supportedTypes = Arrays.asList(annot.value()); | ||||
| 		Class<ResponseEntityExceptionHandler> clazz = ResponseEntityExceptionHandler.class; | ||||
| 		Method handleExceptionMethod = clazz.getMethod("handleException", Exception.class, WebRequest.class); | ||||
| 		ExceptionHandler annotation = handleExceptionMethod.getAnnotation(ExceptionHandler.class); | ||||
| 		List<Class<?>> exceptionTypes = Arrays.asList(annotation.value()); | ||||
| 
 | ||||
| 		for (Method method : DefaultHandlerExceptionResolver.class.getDeclaredMethods()) { | ||||
| 			Class<?>[] paramTypes = method.getParameterTypes(); | ||||
| 			if (method.getName().startsWith("handle") && (paramTypes.length == 4)) { | ||||
| 				String name = paramTypes[0].getSimpleName(); | ||||
| 				assertTrue("@ExceptionHandler is missing " + name, supportedTypes.contains(paramTypes[0])); | ||||
| 				assertTrue("@ExceptionHandler is missing " + name, exceptionTypes.contains(paramTypes[0])); | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
|  | @ -130,6 +133,14 @@ public class ResponseEntityExceptionHandlerTests { | |||
| 		testException(ex); | ||||
| 	} | ||||
| 
 | ||||
| 	@Test | ||||
| 	public void missingPathVariable() throws NoSuchMethodException { | ||||
| 		Method method = getClass().getDeclaredMethod("handle", String.class); | ||||
| 		MethodParameter parameter = new MethodParameter(method, 0); | ||||
| 		Exception ex = new MissingPathVariableException("param", parameter); | ||||
| 		testException(ex); | ||||
| 	} | ||||
| 
 | ||||
| 	@Test | ||||
| 	public void missingServletRequestParameter() { | ||||
| 		Exception ex = new MissingServletRequestParameterException("param", "type"); | ||||
|  | @ -241,8 +252,10 @@ public class ResponseEntityExceptionHandlerTests { | |||
| 			headers.set("someHeader", "someHeaderValue"); | ||||
| 			return handleExceptionInternal(ex, "error content", headers, status, request); | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 
 | ||||
| 	@SuppressWarnings("unused") | ||||
| 	void handle(String arg) { | ||||
| 	} | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -16,6 +16,7 @@ | |||
| 
 | ||||
| package org.springframework.web.servlet.mvc.support; | ||||
| 
 | ||||
| import java.lang.reflect.Method; | ||||
| import java.util.Collections; | ||||
| 
 | ||||
| import org.junit.Before; | ||||
|  | @ -36,6 +37,7 @@ import org.springframework.validation.BindException; | |||
| import org.springframework.web.HttpMediaTypeNotSupportedException; | ||||
| import org.springframework.web.HttpRequestMethodNotSupportedException; | ||||
| import org.springframework.web.bind.MethodArgumentNotValidException; | ||||
| import org.springframework.web.bind.MissingPathVariableException; | ||||
| import org.springframework.web.bind.MissingServletRequestParameterException; | ||||
| import org.springframework.web.bind.ServletRequestBindingException; | ||||
| import org.springframework.web.multipart.support.MissingServletRequestPartException; | ||||
|  | @ -93,6 +95,19 @@ public class DefaultHandlerExceptionResolverTests { | |||
| 		assertEquals("Invalid Accept header", "application/pdf", response.getHeader("Accept")); | ||||
| 	} | ||||
| 
 | ||||
| 	@Test | ||||
| 	public void handleMissingPathVariable() throws NoSuchMethodException { | ||||
| 		Method method = getClass().getMethod("handle", String.class); | ||||
| 		MethodParameter parameter = new MethodParameter(method, 0); | ||||
| 		MissingPathVariableException ex = new MissingPathVariableException("foo", parameter); | ||||
| 		ModelAndView mav = exceptionResolver.resolveException(request, response, null, ex); | ||||
| 		assertNotNull("No ModelAndView returned", mav); | ||||
| 		assertTrue("No Empty ModelAndView returned", mav.isEmpty()); | ||||
| 		assertEquals("Invalid status code", 500, response.getStatus()); | ||||
| 		assertEquals("Missing URI template variable 'foo' for method parameter of type String", | ||||
| 				response.getErrorMessage()); | ||||
| 	} | ||||
| 
 | ||||
| 	@Test | ||||
| 	public void handleMissingServletRequestParameter() { | ||||
| 		MissingServletRequestParameterException ex = new MissingServletRequestParameterException("foo", "bar"); | ||||
|  | @ -196,6 +211,7 @@ public class DefaultHandlerExceptionResolverTests { | |||
| 		assertSame(ex, request.getAttribute("javax.servlet.error.exception")); | ||||
| 	} | ||||
| 
 | ||||
| 	@SuppressWarnings("unused") | ||||
| 	public void handle(String arg) { | ||||
| 	} | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue