Fix issue in AnnotationMethodHandlerExceptionResolver

Caching of resovled exceptions introduced in SPR-7703 also introduced a
side effect whereby if exactly one exception was previously cached, any
other exception would appear as a match to the previously matched
@ExceptionHandler method.

This change ensures use of a fresh map when determining matching
@ExceptionHandler methods while also updating the cache.

Issue: SPR-9209

spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java

@@ -25,6 +25,7 @@ import java.lang.reflect.Method;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -147,7 +148,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 			exceptionHandlerCache.put(handlerType, handlers);
 		}
 
-		final Map<Class<? extends Throwable>, Method> resolverMethods = handlers;
+		final Map<Class<? extends Throwable>, Method> matchedHandlers = new HashMap<Class<? extends Throwable>, Method>();
 
 		ReflectionUtils.doWithMethods(handlerType, new ReflectionUtils.MethodCallback() {
 			public void doWith(Method method) {
@@ -155,11 +156,11 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 				List<Class<? extends Throwable>> handledExceptions = getHandledExceptions(method);
 				for (Class<? extends Throwable> handledException : handledExceptions) {
 					if (handledException.isAssignableFrom(thrownExceptionType)) {
-						if (!resolverMethods.containsKey(handledException)) {
-							resolverMethods.put(handledException, method);
+						if (!matchedHandlers.containsKey(handledException)) {
+							matchedHandlers.put(handledException, method);
 						}
 						else {
-							Method oldMappedMethod = resolverMethods.get(handledException);
+							Method oldMappedMethod = matchedHandlers.get(handledException);
 							if (!oldMappedMethod.equals(method)) {
 								throw new IllegalStateException(
 										"Ambiguous exception handler mapped for " + handledException + "]: {" +
@@ -171,7 +172,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 			}
 		});
 
-		handlerMethod = getBestMatchingMethod(resolverMethods, thrownException);
+		handlerMethod = getBestMatchingMethod(matchedHandlers, thrownException);
 		handlers.put(thrownExceptionType, (handlerMethod == null ? NO_METHOD_FOUND : handlerMethod));
 		return handlerMethod;
 	}

spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java

@@ -16,17 +16,20 @@
 
 package org.springframework.web.portlet.mvc.annotation;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.BindException;
 import java.net.SocketException;
+
 import javax.portlet.PortletRequest;
 import javax.portlet.PortletResponse;
 
-import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Test;
-
 import org.springframework.mock.web.portlet.MockRenderRequest;
 import org.springframework.mock.web.portlet.MockRenderResponse;
 import org.springframework.stereotype.Controller;
@@ -106,6 +109,20 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		exceptionResolver.resolveException(request, response, controller, ex);
 	}
 
+	// SPR-9209
+
+	@Test
+	public void cachingSideEffect() {
+		IllegalArgumentException ex = new IllegalArgumentException();
+		SimpleController controller = new SimpleController();
+
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+
+		mav = exceptionResolver.resolveException(request, response, controller, new NullPointerException());
+		assertNull(mav);
+	}
+
 
 	@Controller
 	private static class SimpleController {

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java

@@ -27,6 +27,7 @@ import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -171,7 +172,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 			exceptionHandlerCache.put(handlerType, handlers);
 		}
 
-		final Map<Class<? extends Throwable>, Method> resolverMethods = handlers;
+		final Map<Class<? extends Throwable>, Method> matchedHandlers = new HashMap<Class<? extends Throwable>, Method>();
 
 		ReflectionUtils.doWithMethods(handlerType, new ReflectionUtils.MethodCallback() {
 			public void doWith(Method method) {
@@ -179,11 +180,11 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 				List<Class<? extends Throwable>> handledExceptions = getHandledExceptions(method);
 				for (Class<? extends Throwable> handledException : handledExceptions) {
 					if (handledException.isAssignableFrom(thrownExceptionType)) {
-						if (!resolverMethods.containsKey(handledException)) {
-							resolverMethods.put(handledException, method);
+						if (!matchedHandlers.containsKey(handledException)) {
+							matchedHandlers.put(handledException, method);
 						}
 						else {
-							Method oldMappedMethod = resolverMethods.get(handledException);
+							Method oldMappedMethod = matchedHandlers.get(handledException);
 							if (!oldMappedMethod.equals(method)) {
 								throw new IllegalStateException(
 										"Ambiguous exception handler mapped for " + handledException + "]: {" +
@@ -195,7 +196,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 			}
 		});
 
-		handlerMethod = getBestMatchingMethod(resolverMethods, thrownException);
+		handlerMethod = getBestMatchingMethod(matchedHandlers, thrownException);
 		handlers.put(thrownExceptionType, (handlerMethod == null ? NO_METHOD_FOUND : handlerMethod));
 		return handlerMethod;
 	}

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java

@@ -110,7 +110,7 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		assertEquals("Invalid view name returned", "GenericError", mav.getViewName());
 		assertEquals("Invalid status code returned", 500, response.getStatus());
 	}
-	
+
 	@Test(expected = IllegalStateException.class)
 	public void ambiguous() {
 		IllegalArgumentException ex = new IllegalArgumentException();
@@ -127,7 +127,7 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		assertTrue("ModelAndView not empty", mav.isEmpty());
 		assertEquals("Invalid response written", "IllegalArgumentException", response.getContentAsString());
 	}
-	
+
 	@Test
 	public void responseBody() throws UnsupportedEncodingException {
 		IllegalArgumentException ex = new IllegalArgumentException();
@@ -139,6 +139,20 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		assertEquals("Invalid response written", "IllegalArgumentException", response.getContentAsString());
 	}
 
+	// SPR-9209
+
+	@Test
+	public void cachingSideEffect() {
+		IllegalArgumentException ex = new IllegalArgumentException();
+		SimpleController controller = new SimpleController();
+
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+
+		mav = exceptionResolver.resolveException(request, response, controller, new NullPointerException());
+		assertNull(mav);
+	}
+
 
 	@Controller
 	private static class SimpleController {